x/build: reduce ambiguity in long test of main Go repo #39054
Labels
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.
Milestone
Comments
dmitshur
added
Builders
|
Change https://golang.org/cl/233898 mentions this issue: |
|
Change https://golang.org/cl/233901 mentions this issue: |
gopherbot
pushed a commit
to golang/build
that referenced
this issue
May 18, 2020
Previously, it was possible to define a builder whose IsLongTest method would report positively, such that it'd test golang.org/x repos in long test mode, but the main Go repository in short test mode. This would be the case for any builder with "-longtest" suffix if it did not manually include GO_TEST_SHORT=0 in its environment configuration. It's unlikely we would want to do that intentionally, so this refactor makes such misconfiguration less likely by inserting the GO_TEST_SHORT environment variable assignment into the output from Env automatically. Now, a longtest builder is defined in one consistent way: it must have a "-longtest" suffix, so that the IsLongTest method reports positively. For golang/go#39054. For golang/go#29252. For golang/go#12508. Change-Id: Ic24df7b3b7de7db728bba6dc6ad4dd38a2e61e82 Reviewed-on: https://go-review.googlesource.com/c/build/+/233901 Reviewed-by: Carlos Amedee <[email protected]> Reviewed-by: Alexander Rakoczy <[email protected]>
|
Change https://golang.org/cl/234531 mentions this issue: |
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Jun 2, 2020
The goal of this change is to reduce the chance of issuing a release with an unintended regression that would be caught by running long tests. This change adds long tests that are run during the -prepare step, in addition to all the existing short tests that are run. Executing the long tests is implemented by adding two new test-only release targets. For a release to be considered complete, all release targets must complete. These test-only targets are built only for the purpose of verifying their tests succeed, or to block the release otherwise. They do not produce release artifacts. The new test-only targets are named after the builder which is used to perform their tests, and they are: • linux-amd64-longtest • windows-amd64-longtest More builders may be added in the future, but care must be taken to ensure the test execution environment is as close as possible to that of build.golang.org post-submit builders, in order to avoid false positives and false negatives. As part of a gradual rollout, this change also adds a flag to skip longtest builders. It's meant to be used in case a long test proves to be flaky, and enough confidence can be gained through testing elsewhere that the failure is not a regression caused by a change merged to the release branch. For now, its default value includes both longtest builders, so they are currently opt-in and this CL is a no-op. After testing proves that it is viable to rely on this (and any issues preventing that from being possible are resolved), the default value of the flag will be changed to the empty string. For golang/go#29252. For golang/go#39054. For golang/go#37827. Fixes golang/go#24614. Change-Id: I33ea6601aade2873f857c63f00a0c11821f35a95 Reviewed-on: https://go-review.googlesource.com/c/build/+/234531 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> Reviewed-by: Alexander Rakoczy <[email protected]>
gopherbot
pushed a commit
that referenced
this issue
Aug 18, 2020
There were two places where the -short flag was added in order to speed up tests when run in short mode, in CL 178399 and CL 177417. It appears viable to re-use the GO_TEST_SHORT value so that -short flag is not used when the tests are executed on a longtest builder, where it is not a goal to skip slow tests for improved performance. Do so, in order to make the testing configurations simpler and more predictable. Factor out the flag name out of the string returned by short, so that it can be used in context of 'go test' which can accept a -short flag, and a test binary which requires the use of a longer -test.short flag. For #39054. For #29252. Change-Id: I52dfbef73cc8307735c52e2ebaa609305fb05933 Reviewed-on: https://go-review.googlesource.com/c/go/+/233898 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Taking into account the build infrastructure at build.golang.org, local development, and issues such as #26473, #34707, I understand we currently have the goal of having good support for two well-known testing configurations (for a given GOOS/GOARCH pair) for the main Go repository:
all.bashgo test -short-longtestpost-submit builders (e.g.,linux-amd64-longtest)While investigating #29252, I found that there is some ambiguity in what it means to test the main Go repo in long mode. It's not easy to say "in order to run a long test, do this" and have a predictable outcome. We currently say "run
all.bashandgo test std cmd" in some places, but there's room for improvement.We want to ensure long tests are passing for Go releases. To support that goal, I think it will helpful to reduce ambiguity in what it means to run a long test on the Go repo.
This is a high level tracking issue for improvements in this area, and for any discussion that may need to happen.
/cc @andybons @cagedmantis @toothrot @golang/osp-team @bradfitz @rsc
The text was updated successfully, but these errors were encountered: