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.

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

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

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

Comments

@gopherbot
Copy link

@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
Copy link
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
… 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 <thepudds1460@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462596
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Tim King <taking@google.com>
TryBot-Bypass: Russ Cox <rsc@golang.org>
@gopherbot
Copy link
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
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 <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link
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
Author

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

gopherbot pushed a commit that referenced this issue Jan 18, 2023
…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 <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/462695
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@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.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Development

No branches or pull requests

3 participants