-
-
Notifications
You must be signed in to change notification settings - Fork 17
fixes False positive with test builder pattern #54
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a false positive issue in the paralleltest linter where test functions using builder patterns to generate test functions were incorrectly flagged as missing parallel calls, even when the returned functions properly called t.Parallel().
- Added support for analyzing function calls that return test functions (builder pattern)
- Enhanced the analyzer to inspect returned function literals for
t.Parallel()calls - Added comprehensive test cases covering both compliant and non-compliant builder patterns
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/paralleltest/paralleltest.go | Implements new logic to handle builder function pattern analysis by adding checkBuilderFunctionForParallel method |
| pkg/paralleltest/testdata/src/t/t_test.go | Adds test cases to verify the fix works correctly for both compliant and non-compliant builder patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if hasParallel { | ||
| return false | ||
| } | ||
|
|
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This early return check is redundant since the same condition is checked at line 306 with return !hasParallel. The early return here prevents unnecessary traversal once parallel is found, but the logic at line 306 should be return true to continue inspection when parallel is not found.
| if hasParallel { | |
| return false | |
| } |
Issue: #50