dashboard: catch potential pitfall in TestBuilderConfig
Previously, it was possible to accidentally try to specify the
branch name for the Go repository by writing something like:
{b("nacl-386", "go@go1.13"), none},
That looks like it would test the right thing,
but in reality it would be equivalent to:
{b("nacl-386@master", "go"), none},
Because the branch of the builder always overwrites the branch
of the "go" repository.
This is not intuitive and easy to miss when reviewing or reading
the code.
Add a check that detects and panics on this b() API pitfall,
so that it is not accidentally introduced in the future, and
so that code readers don't need to spend time verifying that
none of the existing test cases have fallen victim to it.
Updates golang/go#34738
Change-Id: Id2986e88c0e3585eaa7b45f33de97e1902847305
Reviewed-on: https://go-review.googlesource.com/c/build/+/199817
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/dashboard/builders_test.go b/dashboard/builders_test.go
index 4d4ee47..edd2ef9 100644
--- a/dashboard/builders_test.go
+++ b/dashboard/builders_test.go
@@ -222,8 +222,8 @@
branch string
goBranch string
}
- // builder may end in "@go1.N" (as alias for "@release-branch.go1.N") or "@branch-name".
- // repo may end in "@1.N" (as alias for "@release-branch.go1.N")
+ // builder may end in "@go1.N" or "@1.N" (as alias for "@release-branch.go1.N") or "@branch-name".
+ // repo (other than "go") may end in "@go1.N" or "@1.N" (as alias for "@release-branch.go1.N").
b := func(builder, repo string) builderAndRepo {
br := builderAndRepo{
testName: builder + "," + repo,
@@ -241,6 +241,9 @@
f := strings.SplitN(repo, "@", 2)
br.repo = f[0]
br.branch = f[1]
+ if br.repo == "go" {
+ panic(fmt.Errorf(`b(%q, %q): for "go" repo, must use the @%s suffix on the builder, not on the repo`, builder, repo, br.branch))
+ }
}
expandBranch := func(s *string) {
if strings.HasPrefix(*s, "go1.") {
