Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build: use structured (json) test output in CI #59990

Closed
dmitshur opened this issue May 4, 2023 · 6 comments
Closed

x/build: use structured (json) test output in CI #59990

dmitshur opened this issue May 4, 2023 · 6 comments
Assignees
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented May 4, 2023

Issue #37486 tracks adding JSON output to cmd/dist via a -json flag. 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.

@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 4, 2023
@dmitshur dmitshur added this to the Unreleased milestone May 4, 2023
@dmitshur dmitshur self-assigned this May 4, 2023
@dmitshur
Copy link
Contributor Author

dmitshur commented May 10, 2023

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 dist test -list) consist of a set with a pattern "go_test:*" that are automatically generated, one for each package in go list std cmd that has tests, "go_test_bench:*" that are also automatically generated in race mode from packages in go list std cmd list that have benchmarks, and everything else that is a custom registered dist test (e.g., osusergo, pie_internal_cgo, race:runtime/race, etc.).

That property applies to past supported releases (1.20, 1.19) and we will maintain the "go_test:*" pattern as part of completing adding -json to dist test (#37486). It enables a safe split of all dist tests into two parts, one that runs with dist test -run='^go_test:.+$' and the other that runs with dist test -run='!^go_test:.+$' (the ! prefix means to invert the regexp pattern).

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 registerStdTestSpecially in tester.registerTests) and dist no longer includes a "go_test:*" test for them, so these would end up running in both cases. If needed we can work around it with a new internal hook to tell dist "don't register custom tests that are equivalent to what's already covered by go test std cmd".

Since we know the "go_test:*" dist tests can be safely reproduced by doing the equivalent of what dist test does, and use go command directly, we can start get structured output for those tests across all supported Go versions with go test -json std cmd. That covers a majority of tests done in the main Go repo, leaving the others to dist test and to be given the structured output treatment upon completion of #37486.

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.

@gopherbot
Copy link

Change https://go.dev/cl/496181 mentions this issue: cmd/dist: use "pkg[:variant]" as dist test name

@gopherbot
Copy link

Change https://go.dev/cl/496190 mentions this issue: cmd/coordinator: handle new dist test names with current adjust policies

gopherbot pushed a commit to golang/build that referenced this issue May 19, 2023
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>
gopherbot pushed a commit that referenced this issue May 19, 2023
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 dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 20, 2023
@gopherbot
Copy link

Change https://go.dev/cl/497695 mentions this issue: main.star: turn on structured all.bash output

gopherbot pushed a commit to golang/build that referenced this issue May 23, 2023
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>
@dmitshur
Copy link
Contributor Author

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.

@gopherbot
Copy link

Change https://go.dev/cl/499681 mentions this issue: main.star: drop experiments for structured tests

gopherbot pushed a commit to golang/build that referenced this issue Jun 1, 2023
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Projects
Archived in project
Development

No branches or pull requests

2 participants