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,os/signal: TestDetectNohup and TestNohup fail on replacement darwin LUCI builders [1.21 backport] #63911

Closed
gopherbot opened this issue Nov 2, 2023 · 3 comments
Closed

x/build,os/signal: TestDetectNohup and TestNohup fail on replacement darwin LUCI builders [1.21 backport] #63911

gopherbot opened this issue Nov 2, 2023 · 3 comments
Labels
Builders x/build issues (builders, bots, dashboards) CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.21.6

Comments

@gopherbot
Copy link
Contributor

@prattmic requested issue #63875 to be considered for backport to the next 1.21 minor release.

@gopherbot Please backport to 1.20 and 1.21. This test needs to avoid failures on release branches as well.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Nov 2, 2023
@gopherbot gopherbot mentioned this issue Nov 2, 2023
Open
@gopherbot gopherbot added Builders x/build issues (builders, bots, dashboards) compiler/runtime Issues related to the Go compiler and/or runtime. labels Nov 2, 2023
@gopherbot gopherbot modified the milestones: Unreleased, Go1.21.4, Go1.21.5 Nov 2, 2023
@heschi heschi added Testing An issue that has been verified to require only test changes, not just a test failure. CherryPickApproved Used during the release process for point releases labels Nov 8, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Nov 8, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/546021 mentions this issue: [release-branch.go1.21] os/signal: remove go t.Run from TestNohup

@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/546022 mentions this issue: [release-branch.go1.21] os/signal: skip nohup tests on darwin builders

@gopherbot gopherbot modified the milestones: Go1.21.5, Go1.21.6 Dec 5, 2023
@gopherbot
Copy link
Contributor Author

Closed by merging 7dc67e8 to release-branch.go1.21.

gopherbot pushed a commit that referenced this issue Dec 7, 2023
@prattmic @cagedmantis
[release-branch.go1.21] os/signal: remove go t.Run from TestNohup
7dc67e8 
Since CL 226138, TestNohup has a bit of a strange construction: it wants
to run the "uncaught" subtests in parallel with each other, and the
"nohup" subtests in parallel with each other, but also needs join
between "uncaught" and "nohop" so it can Stop notifying for SIGHUP.

It achieves this by doing `go t.Run` with a WaitGroup rather than using
`t.Parallel` in the subtest (which would make `t.Run` return immediately).

However, this makes things more difficult to understand than necessary.
As noted on https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks,
a second layer of subtest can be used to join parallel subtests.

Switch to this form, which makes the test simpler to follow
(particularly the cleanup that goes with "uncaught").

For #63799.
For #63911.

Change-Id: Ibfce0f439508a7cfca848c7ccfd136c9c453ad8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/538899
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit 5622a4b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/546021
gopherbot pushed a commit that referenced this issue Dec 8, 2023
@prattmic @cagedmantis
[release-branch.go1.21] os/signal: skip nohup tests on darwin builders
bbab863 
The new LUCI builders have a temporary limitation that breaks nohup.
Skip nohup tests there.

For #63875.
Fixes #63911.

Cq-Include-Trybots: luci.golang.try:go1.21-darwin-amd64_13
Change-Id: Ia9ffecea7310f84a21f6138d8f8cdfc5e1392307
Reviewed-on: https://go-review.googlesource.com/c/go/+/538698
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit a334c45)
Reviewed-on: https://go-review.googlesource.com/c/go/+/546022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.21.6
Development

No branches or pull requests
2 participants
@gopherbot @heschi