Skip to content

Commit d058254

Browse files
committed
cmd/dist: always include variant in package names
Our attempt to evenly distribute tests across shards struggles a bit because certain long-running targets are very difficult to distinguish in ResultDB, namely racebench and the test directory tests. These are the only tests where the JSON output from dist omits the variant from the package, making it impossible to distinguish them in the test result data. My current suspicion is that this is preventing the load balancing from being effective for the race builders in particular, though I worry the longtest builders have a similar situation with the test directory tests. For #65814. Change-Id: I5804c2af092ff9aa4a3f0f6897b4a57c4628f837 Reviewed-on: https://go-review.googlesource.com/c/go/+/681955 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
1 parent 3254c2b commit d058254

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

src/cmd/dist/test.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,10 @@ type goTest struct {
336336
// omitVariant indicates that variant is used solely for the dist test name and
337337
// that the set of test names run by each variant (including empty) of a package
338338
// is non-overlapping.
339+
//
340+
// TODO(mknyszek): Consider removing omitVariant as it is no longer set to true
341+
// by any test. It's too valuable to have timing information in ResultDB that
342+
// corresponds directly with dist names for tests.
339343
omitVariant bool
340344

341345
// We have both pkg and pkgs as a convenience. Both may be set, in which
@@ -595,8 +599,11 @@ func (t *tester) registerRaceBenchTest(pkg string) {
595599
defer timelog("end", dt.name)
596600
ranGoBench = true
597601
return (&goTest{
598-
variant: "racebench",
599-
omitVariant: true, // The only execution of benchmarks in dist; benchmark names are guaranteed not to overlap with test names.
602+
variant: "racebench",
603+
// Include the variant even though there's no overlap in test names.
604+
// This makes the test targets distinct, allowing our build system to record
605+
// elapsed time for each one, which is useful for load-balancing test shards.
606+
omitVariant: false,
600607
timeout: 1200 * time.Second, // longer timeout for race with benchmarks
601608
race: true,
602609
bench: true,
@@ -983,8 +990,11 @@ func (t *tester) registerTests() {
983990
id := fmt.Sprintf("%d_%d", shard, nShards)
984991
t.registerTest("../test",
985992
&goTest{
986-
variant: id,
987-
omitVariant: true, // Shards of the same Go package; tests are guaranteed not to overlap.
993+
variant: id,
994+
// Include the variant even though there's no overlap in test names.
995+
// This makes the test target more clearly distinct in our build
996+
// results and is important for load-balancing test shards.
997+
omitVariant: false,
988998
pkg: "cmd/internal/testdir",
989999
testFlags: []string{fmt.Sprintf("-shard=%d", shard), fmt.Sprintf("-shards=%d", nShards)},
9901000
runOnHost: true,

0 commit comments

Comments
 (0)