fix: Add skip to test if branch do not exists, instead of failing #9204

rishabh625 · 2022-04-25T12:10:44Z

Signed-off-by: rishabh625 rishabhmishra625@gmail.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

Fixes:- 9203

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

codecov · 2022-04-25T12:37:59Z

Codecov Report

Merging #9204 (b7f8c2b) into master (fdcaa55) will increase coverage by 0.32%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #9204      +/-   ##
==========================================
+ Coverage   45.49%   45.82%   +0.32%     
==========================================
  Files         219      219              
  Lines       25870    25869       -1     
==========================================
+ Hits        11770    11854      +84     
+ Misses      12462    12371      -91     
- Partials     1638     1644       +6

Impacted Files	Coverage Δ
applicationset/services/scm_provider/gitea.go	`78.16% <66.66%> (+1.05%)`	⬆️
applicationset/services/scm_provider/utils.go	`85.14% <100.00%> (+1.24%)`	⬆️
applicationset/services/scm_provider/github.go	`75.29% <0.00%> (-5.89%)`	⬇️
util/cert/cert.go	`82.40% <0.00%> (-2.35%)`	⬇️
...is/applicationset/v1alpha1/applicationset_types.go	`47.22% <0.00%> (ø)`
server/server.go	`54.23% <0.00%> (+0.15%)`	⬆️
util/gpg/gpg.go	`68.20% <0.00%> (+0.20%)`	⬆️
util/cmp/stream.go	`52.14% <0.00%> (+1.09%)`	⬆️
... and 6 more

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 fdcaa55...b7f8c2b. Read the comment docs.

leoluz · 2022-04-25T20:20:25Z

Hi @rishabh625,
I have a PR unrelated to applicationset failing during unit-test with:

--- FAIL: TestGiteaListRepos (11.79s)
    --- FAIL: TestGiteaListRepos/blank_protocol (11.79s)
        gitea_test.go:59: 
            	Error Trace:	gitea_test.go:59
            	Error:      	Expected nil, but got: &errors.errorString{s:"404 Not Found"}
            	Test:       	TestGiteaListRepos/blank_protocol
        gitea_test.go:69: 
            	Error Trace:	gitea_test.go:69
            	Error:      	Should NOT be empty, but was []
            	Test:       	TestGiteaListRepos/blank_protocol
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

goroutine 183 [running]:
testing.tRunner.func1.2({0x3d8[31](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:31)80, 0xc0007bae88})
	/opt/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1389 +0x[36](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:36)6
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1[39](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:39)2 +0x5d2
panic({0x3d83180, 0xc0007bae88})
	/opt/hostedtoolcache/go/1.18.1/x64/src/runtime/panic.go:844 +0x258
github.com/argoproj/argo-cd/v2/applicationset/services/scm_provider.TestGiteaListRepos.func1(0x0?)
	/home/runner/work/argo-cd/argo-cd/applicationset/services/scm_provider/gitea_test.go:70 +0x925
testing.tRunner(0xc0001ebd[40](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:40), 0xc0007b5100)
	/opt/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1[43](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:43)9 +0x214
created by testing.(*T).Run

Just wanted to confirm if this PR is addressing this issue??
Tks

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

rishabh625 · 2022-04-26T05:37:23Z

Hi @rishabh625, I have a PR unrelated to applicationset failing during unit-test with:

--- FAIL: TestGiteaListRepos (11.79s)
    --- FAIL: TestGiteaListRepos/blank_protocol (11.79s)
        gitea_test.go:59: 
            	Error Trace:	gitea_test.go:59
            	Error:      	Expected nil, but got: &errors.errorString{s:"404 Not Found"}
            	Test:       	TestGiteaListRepos/blank_protocol
        gitea_test.go:69: 
            	Error Trace:	gitea_test.go:69
            	Error:      	Should NOT be empty, but was []
            	Test:       	TestGiteaListRepos/blank_protocol
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

goroutine 183 [running]:
testing.tRunner.func1.2({0x3d8[31](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:31)80, 0xc0007bae88})
	/opt/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1389 +0x[36](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:36)6
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1[39](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:39)2 +0x5d2
panic({0x3d83180, 0xc0007bae88})
	/opt/hostedtoolcache/go/1.18.1/x64/src/runtime/panic.go:844 +0x258
github.com/argoproj/argo-cd/v2/applicationset/services/scm_provider.TestGiteaListRepos.func1(0x0?)
	/home/runner/work/argo-cd/argo-cd/applicationset/services/scm_provider/gitea_test.go:70 +0x925
testing.tRunner(0xc0001ebd[40](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:40), 0xc0007b5100)
	/opt/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1[43](https://github.com/argoproj/argo-cd/runs/6162315396?check_suite_focus=true#step:15:43)9 +0x214
created by testing.(*T).Run

Just wanted to confirm if this PR is addressing this issue?? Tks

Hi @leoluz : yeah this is addressing issue that u mentioned above, wrt gitea failing to list default branch for test-ldap-repos

rishabh625 · 2022-04-26T06:51:24Z

@jannfis @leoluz @crenshaw-dev : can u please review?

crenshaw-dev

I think adding a unit test for this would make sense.

crenshaw-dev · 2022-04-26T12:54:36Z

applicationset/services/scm_provider/utils.go

@@ -126,6 +126,9 @@ func getBranches(ctx context.Context, provider SCMProviderService, repos []*Repo
 	for _, repo := range repos {
 		reposFilled, err := provider.GetBranches(ctx, repo)
 		if err != nil {
+			if checkBranchDoNotExist(err.Error()) {


Would this hide an error where the repo doesn't exist? Or will it only return true if a branch doesn't exist?

It will only skip errors if branch doesn't exist, i'll add a UT for this

Added Test cases, added with gitea provider only

branch doesn't exist

repo doesn't exist

applicationset/services/scm_provider/utils.go

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

applicationset/services/scm_provider/utils.go

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

crenshaw-dev · 2022-04-27T14:07:43Z

applicationset/services/scm_provider/utils.go

+			if checkBranchDoNotExist(err) {
+				continue
+			}


After reviewing the test, I don't think this is the correct fix. We're hitting an error because the gitea API is returning main as the default branch name for this repo. But that repo doesn't have a main branch. It looks like a bug in the gitea API.

In the case of a 404 we could fall back to listing all branches and then picking the 1st one if there's only one. I'll put up another PR based on this one.

rishabh625 · 2022-04-27T18:02:09Z

Closing as fixed in #9228

fix: Add skip to test if branch do not exists, instead of failing

fc2d87f

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

rishabh625 force-pushed the fix-git-test-if-branch-donotexist branch from 61ca0bd to fc2d87f Compare April 25, 2022 12:21

leoluz self-requested a review April 25, 2022 20:21

Retrigger CI pipeline

5fba4d0

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

rishabh625 mentioned this pull request Apr 26, 2022

fix: ensure appset git generator works with private repo #9179

Merged

10 tasks

crenshaw-dev requested changes Apr 26, 2022

View reviewed changes

leoluz requested changes Apr 26, 2022

View reviewed changes

applicationset/services/scm_provider/utils.go Outdated Show resolved Hide resolved

Added test cases

a893db9

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

danielhelfand mentioned this pull request Apr 26, 2022

chore: update repo client to use updated methods #9208

Merged

10 tasks

Returned error from gitea implementation

5b423a3

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

rishabh625 requested review from leoluz and crenshaw-dev April 27, 2022 02:57

pasha-codefresh requested changes Apr 27, 2022

View reviewed changes

applicationset/services/scm_provider/utils.go Show resolved Hide resolved

added comment changes

b7f8c2b

Signed-off-by: rishabh625 <rishabhmishra625@gmail.com>

rishabh625 requested a review from pasha-codefresh April 27, 2022 11:54

pasha-codefresh approved these changes Apr 27, 2022

View reviewed changes

crenshaw-dev requested changes Apr 27, 2022

View reviewed changes

rishabh625 closed this Apr 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Add skip to test if branch do not exists, instead of failing #9204

fix: Add skip to test if branch do not exists, instead of failing #9204

rishabh625 commented Apr 25, 2022

codecov bot commented Apr 25, 2022 •

edited

Loading

leoluz commented Apr 25, 2022 •

edited

Loading

rishabh625 commented Apr 26, 2022

rishabh625 commented Apr 26, 2022

crenshaw-dev left a comment

crenshaw-dev Apr 26, 2022

rishabh625 Apr 26, 2022

rishabh625 Apr 26, 2022 •

edited

Loading

crenshaw-dev Apr 27, 2022

rishabh625 commented Apr 27, 2022

fix: Add skip to test if branch do not exists, instead of failing #9204

fix: Add skip to test if branch do not exists, instead of failing #9204

Conversation

rishabh625 commented Apr 25, 2022

codecov bot commented Apr 25, 2022 • edited Loading

Codecov Report

leoluz commented Apr 25, 2022 • edited Loading

rishabh625 commented Apr 26, 2022

rishabh625 commented Apr 26, 2022

crenshaw-dev left a comment

Choose a reason for hiding this comment

crenshaw-dev Apr 26, 2022

Choose a reason for hiding this comment

rishabh625 Apr 26, 2022

Choose a reason for hiding this comment

rishabh625 Apr 26, 2022 • edited Loading

Choose a reason for hiding this comment

crenshaw-dev Apr 27, 2022

Choose a reason for hiding this comment

rishabh625 commented Apr 27, 2022

codecov bot commented Apr 25, 2022 •

edited

Loading

leoluz commented Apr 25, 2022 •

edited

Loading

rishabh625 Apr 26, 2022 •

edited

Loading