Redesign paralleltest to be more agnostic of walks to handle fixtures and helpers #58

thespags · 2025-12-01T05:27:39Z

This is a significant redesign to solve issues #56 and #49.

There are three flavors of test run, literal, func, builder.
We treat them as generically as possible to walk any node looking for t.Parallel, t.Run, t.Setenv, plus t.Chdir.
This allows us to identify those calls in helper functions.

With go1.22, I've verified that the range check is no longer needed; it may be nice to support older versions, but removing the checks streamlines the redesign.

Additionally, we only report defer issues if there is a nested t.Run (and the container t.Parallel), otherwise we don't need to use t.Cleanup.

I understand if this is too drastic, and I can look at supporting the fork.

… and helpers.

thespags · 2025-12-01T05:27:59Z

pkg/paralleltest/testdata/src/cleanup/cleanup_test.go

-	defer fmt.Println("cleanup 1") // want "Function TestWithMultipleDefersAndParallel uses defer with t.Parallel, use t.Cleanup instead to ensure cleanup runs after parallel subtests complete"
-	defer fmt.Println("cleanup 2") // want "Function TestWithMultipleDefersAndParallel uses defer with t.Parallel, use t.Cleanup instead to ensure cleanup runs after parallel subtests complete"
-	defer fmt.Println("cleanup 3") // want "Function TestWithMultipleDefersAndParallel uses defer with t.Parallel, use t.Cleanup instead to ensure cleanup runs after parallel subtests complete"
+	defer fmt.Println("cleanup 1")


There are no subtests, so this defer is unnecessary.

thespags · 2025-12-01T05:28:10Z

pkg/paralleltest/testdata/src/t/chdir_test.go

+	"testing"
+)
+
+func TestFunctionWithChdir(t *testing.T) {


Same as setenv, but with chdir.

thespags · 2025-12-01T05:30:05Z

pkg/paralleltest/testdata/src/t/t_test.go

+}
+
+func TestNestedFunctionWithParallel(t *testing.T) {
+	nestedHelper1(nestedParallel(t))


Really odd case, but to show how we can traverse nodes generally.

thespags · 2025-12-01T05:31:39Z

pkg/paralleltest/testdata/src/t/t_test.go

 	t.Parallel()
-	t.Run("1", builderWithoutParallel()) // want "Function TestBuilderFunctionMissingParallel missing the call to method parallel in the test run"
-	t.Run("2", builderWithoutParallel()) // want "Function TestBuilderFunctionMissingParallel missing the call to method parallel in the test run"
+	t.Run("1", builderWithoutParallel()) // want "Function builderWithoutParallel missing the call to method parallel in the t.Run\n"


This is a little bit of a cop out, we need to carry the container of t.Run to report TestBuilderFunctionMissingParallel; here, we return "literal" if it's inline, or the called method. It feels more subjective on what is the best approach.

… not parallelable.

thespags · 2025-12-03T05:43:00Z

Additional commit to handle custom methods to mark as not parallelizable.

I've been using temporal and I've run into some non determinism when running the tests. I want to be able skip those from the linter. Instead of hardcoding it, I was following a little of what wrapcheck does to check signatures.

thespags · 2025-12-03T05:45:01Z

Again, @kunwardeep, I understand this is a major refactor and may be too much. If so, I can stick to maintaining a fork.

Copilot

Pull request overview

This PR redesigns the paralleltest analyzer to be more generic in detecting t.Parallel(), t.Run(), t.Setenv(), and t.Chdir() calls across different test patterns (literal functions, function references, and builder functions). The redesign removes Go version-specific range checks that are no longer needed in Go 1.22+ and refines the defer/cleanup reporting to only flag issues when there are nested t.Run() calls.

Key changes:

Introduces a unified analysis approach that handles inline functions, direct function references, and builder patterns
Removes loop variable reinitialization checks (no longer needed in Go 1.22+)
Adds support for detecting t.Chdir() calls that prevent parallelization
Refines defer statement warnings to only trigger when there are nested subtests

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/paralleltest/util.go	New utility file with helper functions for AST node hashing, test function detection, and method call identification
pkg/paralleltest/paralleltest.go	Complete redesign of analyzer logic to support generic pattern detection and configuration-based setup
pkg/paralleltest/paralleltest_test.go	Updated tests to use new Config-based API instead of flag-based configuration
pkg/paralleltest/testdata/src/t/t_test.go	Updated test expectations to reflect new error message format and removed range-related warnings
pkg/paralleltest/testdata/src/t/setenv_test.go	New test file consolidating Setenv-related test cases with helper function detection
pkg/paralleltest/testdata/src/t/chdir_test.go	New test file for Chdir call detection patterns
pkg/paralleltest/testdata/src/skip/extra_sigs_test.go	New test file for ExtraSigs configuration feature
pkg/paralleltest/testdata/src/ignoremissingsubtests/ignoremissingsubtests_test.go	Removed range variable reinitialization checks
pkg/paralleltest/testdata/src/i/i_test.go	Removed range variable reinitialization checks
pkg/paralleltest/testdata/src/cleanup/cleanup_test.go	Updated defer warnings to only trigger with nested subtests
main.go	Added Viper-based configuration file support
go.mod	Updated Go version to 1.24.0 and added new dependencies
README.md	Updated documentation to reflect configuration file approach and removal of loop variable checks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-03T06:01:19Z

go.mod

+	github.com/spf13/cast v1.10.0 // indirect
+	github.com/spf13/pflag v1.0.10 // indirect
+	github.com/subosito/gotenv v1.6.0 // indirect
+	go.yaml.in/yaml/v3 v3.0.4 // indirect


The import path should be 'gopkg.in/yaml.v3' not 'go.yaml.in/yaml/v3'. This appears to be a typo in the module path.

Suggested change

go.yaml.in/yaml/v3 v3.0.4 // indirect

gopkg.in/yaml.v3 v3.0.4 // indirect

Copilot · 2025-12-03T06:01:19Z

README.md

+# IgnoreMissingSubtests check that missing calls to t.Parallel in subtests are not reported, default true
+ignoreMissingSubtests: false
+# CheckCleanup check that defer is not used with t.Parallel (use t.Cleanup instead), default false
+checkClean: false


Corrected spelling of 'checkClean' to 'checkCleanup' to match the field name in the Config struct.

Suggested change

checkClean: false

checkCleanup: false

IlyasYOY · 2025-12-03T15:16:12Z

@thespags Wow! This looks interesting.

I'd like to mention that the changes in the configuration logic will break the backward compatibility with: https://golangci-lint.run/docs/linters/configuration/#paralleltest. That might need to be fixed separately.

thespags · 2025-12-03T21:42:10Z

@IlyasYOY yeah, this is something that would be v2 and not backwards compatible. As I stop checking for loop variables too.

kunwardeep · 2025-12-03T22:40:11Z

Thanks for your help, but given that this will break backward compatibility, I would suggest starting a fork for your needs.

I am not saying I am not going to do a V2, but I need some time to understand better, and time is a bit scarce atm.

thespags · 2025-12-03T23:31:27Z

given that this will break backward compatibility,

I can add back the options via flags to be compatible and support both yaml and flags. Although I'm reluctant to add back the loop checking with 1.22

thespags · 2025-12-04T05:25:43Z

@kunwardeep I added the flags back to support backwards compatibility. ignoreLoopVar still exists as a flag but is marked as deprecated.

Redesign paralleltest to be more agnostic of walks to handle fixtures…

e16c1c6

… and helpers.

thespags commented Dec 1, 2025

View reviewed changes

pkg/paralleltest/testdata/src/t/chdir_test.go

"testing"

)

func TestFunctionWithChdir(t *testing.T) {

Copy link

Author

thespags Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as setenv, but with chdir.

thespags commented Dec 1, 2025

View reviewed changes

Adjust configuration use yaml, and take an extra sigs to be marked as…

e0d22ed

… not parallelable.

kunwardeep requested a review from Copilot December 3, 2025 06:00

Copilot AI reviewed Dec 3, 2025

View reviewed changes

IlyasYOY mentioned this pull request Dec 3, 2025

handle t.Parallel in fixtures #56

Open

Add flags back to be backwards compatible.

a8df82e

	go.yaml.in/yaml/v3 v3.0.4 // indirect
	gopkg.in/yaml.v3 v3.0.4 // indirect

Uh oh!

Redesign paralleltest to be more agnostic of walks to handle fixtures and helpers #58

Are you sure you want to change the base?

Redesign paralleltest to be more agnostic of walks to handle fixtures and helpers #58

Uh oh!

Conversation

thespags commented Dec 1, 2025

Uh oh!

thespags Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

thespags Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

thespags Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

thespags Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thespags commented Dec 3, 2025

Uh oh!

thespags commented Dec 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasYOY commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thespags commented Dec 3, 2025

Uh oh!

kunwardeep commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thespags commented Dec 3, 2025

Uh oh!

thespags commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thespags Dec 1, 2025 •

edited

Loading

IlyasYOY commented Dec 3, 2025 •

edited

Loading

kunwardeep commented Dec 3, 2025 •

edited

Loading