Add git clone test on integration test #1682

lunny · 2017-05-05T03:52:58Z

Add git clone test on integration test and in this PR I used github.com/src-d/go-git but it seems it don't support git push.

appleboy · 2017-05-05T04:00:55Z

LGTM

lunny · 2017-05-05T08:52:47Z

But http.Server.Shutdown need go1.8 so the tests failed.

ethantkoenig

It's not clear to me what this test is actually accomplishes. This test doesn't cover any Gitea-related code. It seems to be testing gopkg.in/src-d/go-git.v4 more than anything else.

ethantkoenig · 2017-05-05T12:41:32Z

integrations/git_test.go

You can just do assert.False(t, empty)

lunny · 2017-05-05T13:56:17Z

@ethantkoenig, this test will test git Smart HTTP pull routes in Gitea. You can find them in https://github.com/go-gitea/gitea/blob/master/routers/repo/http.go#L47

lunny · 2017-05-07T02:09:49Z

Maybe we could require Go version >= go1.8 since we used server.Shutdown here.

bkcsoft · 2017-05-08T07:34:53Z

integrations/git_test.go

Why not use a temporary Port? using s.URL to get the result?

great idea. I will try.

@bkcsoft It seems there is no s.URL.

Here's how that's done: https://gist.github.com/xcsrz/538e291d12be6ee9a8c7#file-server-go-L13-L19

bkcsoft · 2017-07-03T18:31:11Z

Not sure if this one is still active.

Why are we importing src-d/go-git only for tests? why not use exec.Command and skip some 100+ vendored files ? (here's some inspiration: https://gitlab.com/gitlab-org/gitaly/blob/master/internal/service/ssh/receive_pack_test.go#L109-144 but obviously less Gitaly-specific env-vars 😂 )

lunny · 2017-07-04T01:28:44Z

@bkcsoft via src-d/go-git we will reduce git command dependencies. At the beginning of Gogs, we want to create a pure Go git library that is go-gitea/git. So if there is pure Go library, I would like to use it. Any idea?

bkcsoft · 2017-07-04T17:34:03Z

@lunny In that case we should use src-d/go-git in gitea/git, not here.

But if you want a proper end-to-end test you really need to run git clone/pull/push to see that everything will work correctly for the users 🙂

lunny · 2017-07-05T01:32:15Z

@bkcsoft reasonable! I will change this PR.

lunny · 2017-11-01T05:41:52Z

@bkcsoft done and rebased.

codecov-io · 2017-11-01T05:48:42Z

Codecov Report

Merging #1682 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1682   +/-   ##
=======================================
  Coverage   26.84%   26.84%           
=======================================
  Files          89       89           
  Lines       17608    17608           
=======================================
  Hits         4727     4727           
  Misses      12195    12195           
  Partials      686      686

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eecaba2...0112f7e. Read the comment docs.

bkcsoft · 2017-11-01T14:28:23Z

integrations/git_test.go

Why not use this? https://golang.org/pkg/io/ioutil/#TempDir
That way we don't get collisions if ran concurrently.

bkcsoft · 2017-11-01T14:28:32Z

integrations/git_test.go

I think it needs both (remove before clone - just in case it exists and defer to delete it also after)

Not if we use ioutil.TempDir() :)

lunny · 2017-11-01T15:27:20Z

@bkcsoft done.

bkcsoft · 2017-11-01T20:26:35Z

@lunny Mind having a look at the listen port again? It should be fairly straight forward :)

lafriks · 2017-11-02T05:27:38Z

LGTM

lunny · 2017-11-02T05:49:52Z

@bkcsoft listen port has been changed to random.

lunny added the type/testing label May 5, 2017

lunny added this to the 1.2.0 milestone May 5, 2017

appleboy approved these changes May 5, 2017

View reviewed changes

tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 5, 2017

ethantkoenig suggested changes May 5, 2017

View reviewed changes

lunny added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label May 6, 2017

lunny modified the milestones: 1.3.0, 1.2.0 May 6, 2017

ethantkoenig approved these changes May 6, 2017

View reviewed changes

bkcsoft suggested changes May 8, 2017

View reviewed changes

sapk mentioned this pull request May 19, 2017

Pull request #947 / #1753 (still) breaks HTTPS clone/push/pull to/from "master" when logged in as Admin #1761

Closed

7 tasks

lunny mentioned this pull request Jul 2, 2017

Fix git hooks update to receive required arguments #2095

Merged

lunny modified the milestones: 1.3.0, 1.x.x Oct 11, 2017

lunny force-pushed the lunny/integration_git_clone_test branch from aa9f4c4 to 86a0fb8 Compare November 1, 2017 05:37

lunny changed the title ~~add git clone test on integration test~~ Add git clone test on integration test Nov 1, 2017

lunny removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Nov 1, 2017

lunny modified the milestones: 1.x.x, 1.3.0 Nov 1, 2017

lunny force-pushed the lunny/integration_git_clone_test branch from 86a0fb8 to 6d1f86b Compare November 1, 2017 05:41

bkcsoft reviewed Nov 1, 2017

View reviewed changes

lunny force-pushed the lunny/integration_git_clone_test branch from 6d1f86b to 49d62e0 Compare November 1, 2017 15:27

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 2, 2017

lunny added 6 commits November 2, 2017 13:48

add git clone test on integration test

4604981

use assert.False to simple code

35937bd

remove dependency lib

914c96b

fix git tests

ed598b8

fix git test

03c30e0

use random listen port instead a fixed port

75b6172

lunny force-pushed the lunny/integration_git_clone_test branch from 9d6efb5 to 75b6172 Compare November 2, 2017 05:48

bkcsoft approved these changes Nov 2, 2017

View reviewed changes

bkcsoft added 2 commits November 2, 2017 16:12

Merge branch 'master' into lunny/integration_git_clone_test

9972941

Merge branch 'master' into lunny/integration_git_clone_test

0112f7e

bkcsoft merged commit f70758d into go-gitea:master Nov 2, 2017

lunny deleted the lunny/integration_git_clone_test branch November 3, 2017 00:43

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Uh oh!

Add git clone test on integration test #1682

Add git clone test on integration test #1682

Uh oh!

Conversation

lunny commented May 5, 2017

Uh oh!

appleboy commented May 5, 2017

Uh oh!

lunny commented May 5, 2017

Uh oh!

ethantkoenig left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lunny commented May 5, 2017

Uh oh!

lunny commented May 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkcsoft commented Jul 3, 2017

Uh oh!

lunny commented Jul 4, 2017

Uh oh!

bkcsoft commented Jul 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lunny commented Jul 5, 2017

Uh oh!

lunny commented Nov 1, 2017

Uh oh!

codecov-io commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lunny commented Nov 1, 2017

Uh oh!

bkcsoft commented Nov 1, 2017

Uh oh!

lafriks commented Nov 2, 2017

Uh oh!

lunny commented Nov 2, 2017

Uh oh!

Uh oh!

lunny commented May 7, 2017 •

edited

Loading

bkcsoft commented Jul 4, 2017 •

edited

Loading

codecov-io commented Nov 1, 2017 •

edited

Loading