x/build/cmd/coordinator: include full set of tests (rather than limited trybot set) when a builder is requested via SlowBots #38279
Labels
Builders
x/build issues (builders, bots, dashboards)
FrozenDueToAge
NeedsFix
The path to resolution is known, but the work has not been done.
Milestone
Comments
dmitshur
added
Builders
|
Change https://golang.org/cl/227779 mentions this issue: |
|
Change https://golang.org/cl/227777 mentions this issue: |
|
Change https://golang.org/cl/227778 mentions this issue: |
This comment has been minimized.
This comment has been minimized.
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Apr 9, 2020
Previously, the behavior of ShouldRunDistTest could be overridden by an individual builder by setting a the shouldRunDistTest field to a custom policy implementation. A pitfall of this approach is that it requires each custom policy to reimplement the default cmd/dist test policy as well. If that isn't done when attempting to make a small adjustment to the default policy, the result is that the default policy is ignored. The fasterTrybots policy, as the name suggests, exists to skip some tests in trybot mode. Due to the problem above, it was causing some builders to run more tests in trybot mode than they would if it weren't applied. For example, the freebsd-amd64-12_0 builder should have been skipping the "api" test in trybot mode, but wasn't. This refactor fixes that by modifying the approach for how individual builders can customize the cmd/dist test behavior. Now, a function distTestAdjust is called with the first parameter specifying the default policy decision. That function can adjust the decision as it sees fit, but otherwise it preserves its initial value in situations where no change is intended to be made. For golang/go#38279. Change-Id: I220248f51abfc8e77195ae0c9134a17f7cac9bc2 Reviewed-on: https://go-review.googlesource.com/c/build/+/227777 Reviewed-by: Alexander Rakoczy <[email protected]>
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Apr 9, 2020
Previously, a builder was considered to be "SlowBot" whenever it was requested via the TRY= syntax, and it wasn't already a part of the default set of trybot builders. This change makes it so that a builder is considered a SlowBot if it was explicitly requested via the TRY= syntax, even if it was already going to run anyway. The goal behind this change is to improve the experience of using SlowBots, by making them more useful and predictable. The UI currently reports that "SlowBots are starting ..." whenever there is a non-zero amount of SlowBots. So if a user requested TRY=linux-amd64, they'd likely see "TryBots are starting ...", which can be misunderstood to mean that SlowBots are not working. Additionally, if a user requested three builders via TRY= syntax, but one of them happened to be a part of the default try set, then the message at the end would be "Extra slowbot builds that ran:" including only 2 builders. At that point, it's hard to tell whether they got the TRY= syntax wrong for one of the builders, or if it was omitted because it's always run anyway. A future usability improvement in golang.org/issue/38279 is to make SlowBots not skip any tests that normal trybots may skip for faster results. This change is in preparation for that. For golang/go#38279. For golang/go#34501. Change-Id: Idb7f1bcb1694fe72f85237976995a94f273c7c16 Reviewed-on: https://go-review.googlesource.com/c/build/+/227778 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Alexander Rakoczy <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Our builder configuration in the
golang.org/x/build/dashboardpackage defines a ShouldRunDistTest function that controls what cmd/dist tests are run for each builder.That function gets one parameter,
isTry bool, which is whether it is a trybot (pre-submit) run. Most builders are configured to skip some slow tests for trybot runs, in order to speed them up. A full set of tests runs on post-submit builder runs.When a builder is explicitly requested via the SlowBots UI, it should run that builder as if it's a post-submit build run and not skip some slow tests that are skipped per normal configuration.
As an example, this would be helpful in CL 227002 where its desirable to be able to run a full set of tests before submitting the CL.
/cc @toothrot @cagedmantis @andybons @randall77
The text was updated successfully, but these errors were encountered: