x/build: use structured (json) test output in CI #59990
Comments
dmitshur
added
Builders
|
To get the benefits for the standard library package tests first, this is the general approach. We rely on the property that dist tests (listed by That property applies to past supported releases (1.20, 1.19) and we will maintain the "go_test:*" pattern as part of completing adding At tip, there's a minor detail that doesn't negatively affect coverage. There'll be a small amount of overlap in that there now some packages that are registered specially (see Since we know the "go_test:*" dist tests can be safely reproduced by doing the equivalent of what dist test does, and use We're also planning to adjust dist test names to produce more consistent test IDs for the remaining special tests in #37486, but are intentionally leaving the "go_test:*" pattern out of scope of that. crrev.com/c/4518341 and go.dev/cl/494018 implement this step. |
|
Change https://go.dev/cl/496181 mentions this issue: |
|
Change https://go.dev/cl/496190 mentions this issue: |
The coordinator has some custom logic for "go_test:*" dist test names involved in partitioning, merging, and sorting them using historical test timing data. We also have many builder-specific dist test adjust policies, and some parts of them have become out of date (e.g., macTestPolicy skips "wiki", a dist test that's long gone). Go 1.21 is simplifying the naming pattern for dist test names, and we want to do that without having to manually update the (many!) existing adjust policies, as well as to avoid breaking or creating a need to re-engineer the existing mechanisms that make "go_test:*" dist test names special. Make that possible by remapping the 'go tool dist test -list' output from the new "pkg[:variant]" format to the previous format (or pass it through unmodified if it's already in the old format) for the purpose of dist test adjust policies only. Also keep the raw unmodified dist test name around since it's needed for invoking said dist tests. For golang/go#37486. For golang/go#59990. Change-Id: Ie958609de2a810fa0053a00091a0c54c3f8bfd83 Reviewed-on: https://go-review.googlesource.com/c/build/+/496190 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Austin Clements <austin@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
The work to add the -json flag to the 'dist test' command also cleaned how dist tests are tracked and registered. By now, a pair of (import path, variant) strings is sufficient to uniquely identify every dist test that exists. Some of the custom dist test names have been improved along the way. And since the names are already changing a little anyway, we use this opportunity to make them more uniform and predictable. The mapping from the old dist test names to the new is as follows: - "go_test:pkg" → "pkg" (this is the most common case) - "go_test_bench:pkg" → "pkg:racebench" - all other custom names are now called "pkg:variant", where variant is a description of their test configuration and pkg is the import path of the Go package under test CL 495016 introduced test variants and used variant names for rewriting the Package field in JSON events, and now that same name starts to also be used as the dist test name. Like previously done in CL 494496, registering a test variant involving multiple Go packages creates a "pkg:variant" dist test name for each. In the future we may combine their 'go test' invocation purely as an optimization. We can do this with the support of CL 496190 that keeps the coordinator happy and capable of working with both new and old names. In the end, all dist tests now have a consistent "pkg[:variant]" name. For #37486. For #59990. Change-Id: I7eb02a42792a9831a2f3eeab583ff635d24269e8 Co-authored-by: Austin Clements <austin@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/496181 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
dmitshur
added
NeedsFix
|
Change https://go.dev/cl/497695 mentions this issue: |
It worked in https://ci.chromium.org/b/8780270441624243121! Turn it on everywhere as the next step. For golang/go#59990. Change-Id: I341c86f49b41c6784b19ce546814c8d359e26e57 Reviewed-on: https://go-review.googlesource.com/c/build/+/497695 TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
|
This has been flipped on and running in LUCI since last week. Some examples:
The relevant part shows up at the top of the page, under the "Failed Tests" section. Switching to the Test Results tab (or clicking the "View All Tests" link) shows only the structured test results. crrev.com/c/4549679 and go.dev/cl/497695 started to use "dist test -json" at tip. We were able to simplify the dist test naming pattern and drop the "go_test:" prefix via CL 496181 and CL 496190. The aforementioned plan for having structured output for the "go test std cmd" subset of all.bash still applies to older supported releases branches. Minor follow ups and clean up that will happen can be tracked in individual issues. There are no plans to also add this support to the old coordinator. I think we can call this done. |
|
Change https://go.dev/cl/499681 mentions this issue: |
The experiment has graduated to default behavior in crrev.com/c/4558786 and can be removed here. (This undoes the change applied in CL 497695.) For golang/go#59990. Change-Id: Ie49407efbcb2641a61ad992e2443e6bd30057730 Reviewed-on: https://go-review.googlesource.com/c/build/+/499681 TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Issue #37486 tracks adding JSON output to
cmd/distvia a-jsonflag. This is the corresponding x/build issue, to track the work of starting to use that functionality in our CI system.I have a plan for the "##### Testing packages." portion of all.bash that I'll describe below. It should be possible to gradually start migrating there first—which will benefit every test run that happens to have a failing test in the Go standard library—before getting to the rest (the smaller, harder remaining parts of #37486 that aren't quite ready yet).
CC @aclements, @golang/release, @mknyszek.
The text was updated successfully, but these errors were encountered: