Skip to content

cmd/vet: panic in new parallel subtest check when t.Run has single argument [1.20 backport] #57911

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
Closed
gopherbot opened this issue Jan 18, 2023 · 5 comments
Closed

cmd/vet: panic in new parallel subtest check when t.Run has single argument [1.20 backport] #57911

gopherbot opened this issue Jan 18, 2023 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Milestone
Go1.20

Comments

@gopherbot
Copy link
Contributor

@ianlancetaylor requested issue #57908 to be considered for backport to the next 1.20 minor release.

@gopherbot Please open backport to 1.20.

Seems like this affects the 1.20 release.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jan 18, 2023
@gopherbot gopherbot mentioned this issue Jan 18, 2023
Closed
@ianlancetaylor ianlancetaylor added this to the Go1.20 milestone Jan 18, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/462596 mentions this issue: go/analysis/passes/loopclosure: avoid panic in new parallel subtest check when t.Run has single argument

@heschi heschi added the CherryPickApproved Used during the release process for point releases label Jan 18, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jan 18, 2023
@heschi
Copy link
Contributor

heschi commented Jan 18, 2023

Approving; any CL that meets the usual criteria for submission at this point in the cycle is fine to "backport". But also, feel free to add it to #57854 rather than filing a backport issue.

gopherbot pushed a commit to golang/tools that referenced this issue Jan 18, 2023
@thepudds @rsc
[internal-branch.go1.20-vendor] go/analysis/passes/loopclosure: avoid…
070db29 
… panic in new parallel subtest check when t.Run has single argument

The new Go 1.20 parallel subtest check in the loopclosure pass
can panic in the rare case of t.Run with a single argument:

    fn := func() (string, func(t *testing.T)) { return "", nil }
    t.Run(fn())

A real-world example:

https://github.com/go-spatial/geom/blob/3cd2f5a9a082dd4f827c9f9b69ba5d736d2dcb12/planar/planar_test.go#L118

This has been present for multiple months without seeming to
be reported, so presumably this is a rare pattern.

Avoid that panic by checking t.Run has an expected number
of arguments.

Fixes golang/go#57908.
Fixes golang/go#57911.

Change-Id: I5cde60edf624e16bb9f534e435bcd55e63a15647
GitHub-Last-Rev: 24e65a4
GitHub-Pull-Request: #424
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462282
Run-TryBot: thepudds <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462596
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
Reviewed-by: Tim King <[email protected]>
TryBot-Bypass: Russ Cox <[email protected]>
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/462555 mentions this issue: cmd: update x/tools to latest internal Go 1.20 branch

gopherbot pushed a commit that referenced this issue Jan 18, 2023
@rsc @gopherbot
cmd: update x/tools to latest internal Go 1.20 branch
6fcbd03 
Import x/tools as of CL 462596 (070db2996ebe, Jan 18 2022),
to bring in two vet analysis fixes (printf and loopclosure).

For #57911.
Fixes #57903.
Fixes #57904.

Change-Id: I82fe4e9bd56fb8e64394ee8618c155316942a517
Reviewed-on: https://go-review.googlesource.com/c/go/+/462555
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/462695 mentions this issue: [release-branch.go1.20] cmd: update x/tools to latest internal Go 1.20 branch

@gopherbot
Copy link
Contributor Author

Closed by merging 8b34676 to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Jan 18, 2023
@rsc @gopherbot
[release-branch.go1.20] cmd: update x/tools to latest internal Go 1.2…
8b34676 
…0 branch

Import x/tools as of CL 462596 (070db2996ebe, Jan 18 2022),
to bring in two vet analysis fixes (printf and loopclosure).

For #57911.
Fixes #57903.
Fixes #57904.

Change-Id: I82fe4e9bd56fb8e64394ee8618c155316942a517
Reviewed-on: https://go-review.googlesource.com/c/go/+/462555
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/462695
Run-TryBot: Russ Cox <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
@golang golang locked and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
3 participants
@ianlancetaylor @gopherbot @heschi