Skip to content

Commit 76fd6b5

Browse files
committed
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", "[email protected]"), 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 <[email protected]>
1 parent 67e2d13 commit 76fd6b5

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

dashboard/builders_test.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ func TestBuilderConfig(t *testing.T) {
222222
branch string
223223
goBranch string
224224
}
225-
// builder may end in "@go1.N" (as alias for "@release-branch.go1.N") or "@branch-name".
226-
// repo may end in "@1.N" (as alias for "@release-branch.go1.N")
225+
// builder may end in "@go1.N" or "@1.N" (as alias for "@release-branch.go1.N") or "@branch-name".
226+
// repo (other than "go") may end in "@go1.N" or "@1.N" (as alias for "@release-branch.go1.N").
227227
b := func(builder, repo string) builderAndRepo {
228228
br := builderAndRepo{
229229
testName: builder + "," + repo,
@@ -241,6 +241,9 @@ func TestBuilderConfig(t *testing.T) {
241241
f := strings.SplitN(repo, "@", 2)
242242
br.repo = f[0]
243243
br.branch = f[1]
244+
if br.repo == "go" {
245+
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))
246+
}
244247
}
245248
expandBranch := func(s *string) {
246249
if strings.HasPrefix(*s, "go1.") {

0 commit comments

Comments
 (0)