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.

Sign up for GitHub

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

Jump to bottom

x/build: timeout_scale parameter not applied on LUCI builders for x/ repos #65845

Closed
bcmills opened this issue Feb 21, 2024 · 3 comments
Closed

x/build: timeout_scale parameter not applied on LUCI builders for x/ repos #65845

bcmills opened this issue Feb 21, 2024 · 3 comments
Assignees
@mknyszek
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 21, 2024 •
edited
Loading

Go version

86a32d6

Output of go env in your module/workspace:

N/A

What did you do?

Read #65820 (comment).

What did you see happen?

The logs linked from https://ci.chromium.org/ui/p/golang/builders/ci/x_tools-gotip-darwin-amd64-longtest/b8755859092256698353/overview appear to show go test commands run without setting the -timeout flag.

It appears that the LUCI configuration only uses the timeout_scale parameter to set the GO_TEST_TIMEOUT_SCALE environment variable, which is intended to be specific to cmd/dist and should have little to no effect on other commands.
(https://cs.opensource.google/go/x/build/+/luci-config:main.star;l=1206-1208;drc=c0163da662349fff6c2f585cebd08bbd836b8470)

What did you expect to see?

A -timeout flag set according to the timeout_scale parameter of the builder, analogous to what happens on the legacy builders:
https://cs.opensource.google/go/x/build/+/master:cmd/coordinator/buildstatus.go;l=1187-1190;drc=498808697e0db626ec196a9a5290f5f121fa056a (issue #56968)
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Feb 21, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 21, 2024
@bcmills bcmills mentioned this issue Feb 21, 2024
Closed
@bcmills
Copy link
Contributor Author

bcmills commented Feb 21, 2024

It appears that the actual go test arguments are set in a separate binary here:
https://source.chromium.org/chromium/infra/infra/+/main:go/src/infra/experimental/golangbuild/buildspec.go;l=257-270;drc=06da4728c54588bb3068716c3f5cd51f04fc2d07

So I guess that's where the -timeout argument should be added too?

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 21, 2024
@dmitshur dmitshur assigned dmitshur and mknyszek and unassigned dmitshur Feb 21, 2024
@dmitshur dmitshur mentioned this issue Feb 21, 2024
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/565697 mentions this issue: main.star: use the test_timeout_scale property

@dmitshur dmitshur moved this to In Progress in Go Release Feb 21, 2024
gopherbot pushed a commit to golang/build that referenced this issue Feb 21, 2024
@mknyszek
main.star: use the test_timeout_scale property
bea111e 
This change sets the test_timeout_scale property instead of setting
GO_TEST_TIMEOUT_SCALE directly. This property ensures the timeout scale
propagates to all testing contexts, including `go test`, in golangbuild.

This change also allows run mods to influence the test timeout scale of
subrepo builders, since that was previously not explicitly handled, but
led to flaky test timeouts on subrepo builds.

Depends on https://crrev.com/c/5314204. This CL must wait until that
change lands and rolls out.

Fixes golang/go#65845.

Change-Id: I3d38922d64fa225da233206bf53994d4b85843c7
Reviewed-on: https://go-review.googlesource.com/c/build/+/565697
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
TryBot-Bypass: Michael Knyszek <[email protected]>
@mknyszek
Copy link
Contributor

I believe this is now resolved. I verified on an x/tools longtest build that the -timeout flag is being passed as expected.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Feb 21, 2024
This was referenced Feb 21, 2024
Open
Closed
Closed
Closed
Open
@findleyr findleyr mentioned this issue Feb 21, 2024
Closed
@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 Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mknyszek mknyszek

Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @bcmills @gopherbot