Skip to content

Commit 2b1fc2c

Browse files
committed
dashboard: add linux-amd64-longtest-race builder
This builder runs the longtest test with the race detector turned on to identify races in -short=false tests. Fixes golang/go#54630. Change-Id: I6fabaa43522a4294f3a23e98ae0f436d4cc153d8 Reviewed-on: https://go-review.googlesource.com/c/build/+/449196 Reviewed-by: Heschi Kreinick <[email protected]> Run-TryBot: Michael Knyszek <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 1b7bff9 commit 2b1fc2c

File tree

3 files changed

+87
-20
lines changed

3 files changed

+87
-20
lines changed

cmd/coordinator/internal/legacydash/ui.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,15 @@ func builderSubheading(s string) string {
889889
// For race builders it returns the empty string.
890890
func builderSubheading2(s string) string {
891891
if isRace(s) {
892-
return ""
892+
// Remove "race" and just take whatever the third component is after.
893+
var split []string
894+
for _, sc := range strings.Split(s, "-") {
895+
if sc == "race" {
896+
continue
897+
}
898+
split = append(split, sc)
899+
}
900+
s = strings.Join(split, "-")
893901
}
894902
_, secondThird := splitDash(s)
895903
_, third := splitDash(secondThird)

dashboard/builders.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,17 +1032,34 @@ func (c *HostConfig) BuildletBinaryURL(e *buildenv.Environment) string {
10321032
return "https://storage.googleapis.com/" + e.BuildletBucket + "/buildlet." + c.HostArch
10331033
}
10341034

1035+
// IsRace reports whether this is a race builder.
1036+
// A race builder runs tests without the -short flag.
1037+
//
1038+
// A builder is considered to be a race builder
1039+
// if and only if it one of the components of the builder
1040+
// name is "race" (components are delimited by "-").
10351041
func (c *BuildConfig) IsRace() bool {
1036-
return strings.HasSuffix(c.Name, "-race")
1042+
for _, s := range strings.Split(c.Name, "-") {
1043+
if s == "race" {
1044+
return true
1045+
}
1046+
}
1047+
return false
10371048
}
10381049

10391050
// IsLongTest reports whether this is a longtest builder.
10401051
// A longtest builder runs tests without the -short flag.
10411052
//
10421053
// A builder is considered to be a longtest builder
1043-
// if and only if its name ends with "-longtest".
1054+
// if and only if it one of the components of the builder
1055+
// name is "longtest" (components are delimited by "-").
10441056
func (c *BuildConfig) IsLongTest() bool {
1045-
return strings.HasSuffix(c.Name, "-longtest")
1057+
for _, s := range strings.Split(c.Name, "-") {
1058+
if s == "longtest" {
1059+
return true
1060+
}
1061+
}
1062+
return false
10461063
}
10471064

10481065
// OutboundNetworkAllowed reports whether this builder should be
@@ -1863,6 +1880,23 @@ func init() {
18631880
},
18641881
numTryTestHelpers: 4, // Target time is < 15 min for go.dev/issue/42661.
18651882
})
1883+
addBuilder(BuildConfig{
1884+
Name: "linux-amd64-longtest-race",
1885+
HostType: "host-linux-amd64-bullseye",
1886+
Notes: "Debian Bullseye with the race detector enabled and go test -short=false",
1887+
buildsRepo: func(repo, branch, goBranch string) bool {
1888+
b := buildRepoByDefault(repo)
1889+
if repo != "go" && !(branch == "master" && goBranch == "master") {
1890+
// For golang.org/x repos, don't test non-latest versions.
1891+
b = false
1892+
}
1893+
return b
1894+
},
1895+
needsGoProxy: true, // for cmd/go module tests
1896+
env: []string{
1897+
"GO_TEST_TIMEOUT_SCALE=5", // Inherited from the longtest builder.
1898+
},
1899+
})
18661900
addBuilder(BuildConfig{
18671901
Name: "linux-386-longtest",
18681902
HostType: "host-linux-amd64-bullseye",

dashboard/builders_test.go

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -894,24 +894,49 @@ func TestExpectedMacstadiumVMCount(t *testing.T) {
894894
}
895895
}
896896

897-
// Test that we have a longtest builder and
898-
// that its environment configuration is okay.
897+
// Test that we have longtest builders and
898+
// that their environment configurations are okay.
899899
func TestLongTestBuilder(t *testing.T) {
900-
long, ok := Builders["linux-amd64-longtest"]
901-
if !ok {
902-
t.Fatal("we don't have a linux-amd64-longtest builder anymore, is that intentional?")
903-
}
904-
if !long.IsLongTest() {
905-
t.Error("the linux-amd64-longtest builder isn't a longtest builder, is that intentional?")
906-
}
907-
var shortDisabled bool
908-
for _, e := range long.Env() {
909-
if e == "GO_TEST_SHORT=0" {
910-
shortDisabled = true
911-
}
900+
for _, name := range []string{"linux-amd64-longtest", "linux-amd64-longtest-race"} {
901+
name := name
902+
t.Run(name, func(t *testing.T) {
903+
long, ok := Builders[name]
904+
if !ok {
905+
t.Fatalf("we don't have a %s builder anymore, is that intentional?", name)
906+
}
907+
if !long.IsLongTest() {
908+
t.Errorf("the %s builder isn't a longtest builder, is that intentional?", name)
909+
}
910+
var shortDisabled bool
911+
for _, e := range long.Env() {
912+
if e == "GO_TEST_SHORT=0" {
913+
shortDisabled = true
914+
}
915+
}
916+
if !shortDisabled {
917+
t.Errorf("the %s builder doesn't set GO_TEST_SHORT=0, is that intentional?", name)
918+
}
919+
})
912920
}
913-
if !shortDisabled {
914-
t.Error("the linux-amd64-longtest builder doesn't set GO_TEST_SHORT=0, is that intentional?")
921+
}
922+
923+
// Test that we have race builders and
924+
// that their environment configurations are okay.
925+
func TestRaceBuilder(t *testing.T) {
926+
for _, name := range []string{"linux-amd64-race", "linux-amd64-longtest-race"} {
927+
name := name
928+
t.Run(name, func(t *testing.T) {
929+
race, ok := Builders[name]
930+
if !ok {
931+
t.Fatalf("we don't have a %s builder anymore, is that intentional?", name)
932+
}
933+
if !race.IsRace() {
934+
t.Errorf("the %s builder isn't a race builder, is that intentional?", name)
935+
}
936+
if script := race.AllScript(); !strings.Contains(script, "race") {
937+
t.Errorf("the %s builder doesn't use race.bash or race.bat, it uses %s instead, is that intentional?", name, script)
938+
}
939+
})
915940
}
916941
}
917942

0 commit comments

Comments
 (0)