findbuild: Removed test concurrecny
Removed findbuild concurrency and added test delay to prevent resource exhausted error. Changed CLLandingNotFound error HTTP code to 406 to prevent fallback retrying if user specifies CL with unused repo/branch or CL submitted directly through Git. Removed Change-ID test case.
BUG=b/160901711
TEST=unittests, run local
Change-Id: I4d23dc6170fde9afed9f3c9b63d1cf8f01e7cfff
diff --git a/src/cmd/changelog-cli/main_test.go b/src/cmd/changelog-cli/main_test.go
index f3ce013..60e4abd 100644
--- a/src/cmd/changelog-cli/main_test.go
+++ b/src/cmd/changelog-cli/main_test.go
@@ -304,10 +304,6 @@
CL: "1540",
ShouldErr: true,
},
- "invalid cl identifier": {
- CL: "I6cc721e6e61b3863e549045e68c1a2bd363efa0a",
- ShouldErr: true,
- },
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
diff --git a/src/pkg/findbuild/findbuild_test.go b/src/pkg/findbuild/findbuild_test.go
index 2e34cbf..96269e3 100644
--- a/src/pkg/findbuild/findbuild_test.go
+++ b/src/pkg/findbuild/findbuild_test.go
@@ -19,6 +19,7 @@
"fmt"
"net/http"
"testing"
+ "time"
"go.chromium.org/luci/common/api/gerrit"
"golang.org/x/oauth2"
@@ -51,6 +52,15 @@
ShouldFallback bool
ExpectedError string
}{
+ "exponential search range required": {
+ Change: "1740206",
+ GerritHost: externalGerritURL,
+ GitilesHost: externalGitilesURL,
+ FallbackGerritHost: externalFallbackGerritURL,
+ ManifestRepo: externalManifestRepo,
+ OutputBuildNum: "12371.1001.0",
+ ShouldFallback: true,
+ },
"invalid host": {
Change: "3781",
GerritHost: "https://zop-review.googlesource.com",
@@ -143,12 +153,12 @@
ShouldFallback: false,
},
"branch not in manifest": {
- Change: "1592",
- GerritHost: externalGerritURL,
- GitilesHost: externalGitilesURL,
- ManifestRepo: externalManifestRepo,
- ShouldFallback: false,
- ExpectedError: "406",
+ Change: "1592",
+ GerritHost: externalGerritURL,
+ FallbackGerritHost: externalFallbackGerritURL,
+ GitilesHost: externalGitilesURL,
+ ManifestRepo: externalManifestRepo,
+ ExpectedError: "406",
},
"invalid release branch": {
Change: "150",
@@ -159,15 +169,6 @@
ShouldFallback: true,
ExpectedError: "406",
},
- "exponential search range required": {
- Change: "1740206",
- GerritHost: externalGerritURL,
- GitilesHost: externalGitilesURL,
- FallbackGerritHost: externalFallbackGerritURL,
- ManifestRepo: externalManifestRepo,
- OutputBuildNum: "12371.1001.0",
- ShouldFallback: true,
- },
"chromeos branch": {
Change: "2036225",
GerritHost: externalGerritURL,
@@ -181,38 +182,37 @@
httpClient, _ := getHTTPClient()
for name, test := range tests {
- t.Run(name, func(t *testing.T) {
- req := &BuildRequest{
+ req := &BuildRequest{
+ HTTPClient: httpClient,
+ GerritHost: test.GerritHost,
+ GitilesHost: test.GitilesHost,
+ ManifestRepo: test.ManifestRepo,
+ CL: test.Change,
+ }
+ res, err := FindBuild(req)
+ if err != nil && err.HTTPCode() != "404" && test.ShouldFallback {
+ t.Fatalf("test \"%s\" failed:\nexpected not found error, got %v", name, err)
+ }
+ if err != nil && err.HTTPCode() == "404" {
+ fallbackReq := &BuildRequest{
HTTPClient: httpClient,
- GerritHost: test.GerritHost,
+ GerritHost: test.FallbackGerritHost,
GitilesHost: test.GitilesHost,
ManifestRepo: test.ManifestRepo,
CL: test.Change,
}
- res, err := FindBuild(req)
- if err != nil && err.HTTPCode() != "404" && test.ShouldFallback {
- t.Fatalf("expected not found error, got %v", err)
- }
- if err != nil && err.HTTPCode() == "404" {
- fallbackReq := &BuildRequest{
- HTTPClient: httpClient,
- GerritHost: test.FallbackGerritHost,
- GitilesHost: test.GitilesHost,
- ManifestRepo: test.ManifestRepo,
- CL: test.Change,
- }
- res, err = FindBuild(fallbackReq)
- }
- switch {
- case test.ExpectedError == "" && err != nil:
- t.Fatalf("expected no error, got %v", err)
- case test.ExpectedError != "" && err == nil:
- t.Fatalf("expected error code %s, got nil err", test.ExpectedError)
- case test.ExpectedError != "" && err != nil && test.ExpectedError != err.HTTPCode():
- t.Fatalf("expected error code %s, got error code %s", test.ExpectedError, err.HTTPCode())
- case test.ExpectedError == "" && res.BuildNum != test.OutputBuildNum:
- t.Fatalf("expected output %s, got %s", test.OutputBuildNum, res)
- }
- })
+ res, err = FindBuild(fallbackReq)
+ }
+ switch {
+ case test.ExpectedError == "" && err != nil:
+ t.Fatalf("test \"%s\" failed:\nexpected no error, got %v", name, err)
+ case test.ExpectedError != "" && err == nil:
+ t.Fatalf("test \"%s\" failed:\nexpected error code %s, got nil err", name, test.ExpectedError)
+ case test.ExpectedError != "" && err != nil && test.ExpectedError != err.HTTPCode():
+ t.Fatalf("test \"%s\" failed:\nexpected error code %s, got error code %s", name, test.ExpectedError, err.HTTPCode())
+ case test.ExpectedError == "" && res.BuildNum != test.OutputBuildNum:
+ t.Fatalf("test \"%s\" failed:\nexpected output %s, got %s", name, test.OutputBuildNum, res)
+ }
+ time.Sleep(time.Second * 5)
}
}
diff --git a/src/pkg/utils/changelogerror.go b/src/pkg/utils/changelogerror.go
index 59c4d31..f5b415f 100644
--- a/src/pkg/utils/changelogerror.go
+++ b/src/pkg/utils/changelogerror.go
@@ -149,7 +149,7 @@
errStrFmt := "No build was found containing %s."
link := clLink(clID, instanceURL)
return &UtilChangelogError{
- httpCode: "404",
+ httpCode: "406",
header: "No Build Found",
err: fmt.Sprintf(errStrFmt, "CL "+clID),
htmlErr: fmt.Sprintf(errStrFmt, link),
