lint: Enable paralleltest, fix issues #1367
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enable paralleltest to verify that all tests that can be run in parallel are run in parallel.
Fix all issues reported by paralleltest except those that rely on global state. Add `nolint:paralleltest` as needed for those cases.
Codecov Report
@@ Coverage Diff @@
## master #1367 +/- ##
==========================================
- Coverage 98.42% 98.28% -0.15%
==========================================
Files 53 53
Lines 3493 3493
==========================================
- Hits 3438 3433 -5
- Misses 46 50 +4
- Partials 9 10 +1 see 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
What's the motivation for this change? My experience with this linter and marking tests parallel is that it makes the development experience worse for little value. Specifically, the output of go test now has interspersed logs, the test output is more verbose, and it doesn't reduce test time significantly for cpu-bound tests. I'm all for using parallel test for IO bound tests, but I don't think that's the case for all zap tests. |
abhinav
added a commit
to abhinav/zap
that referenced
this pull request
Oct 1, 2023
The PR was merged prematurely. Maintainers are not yet in agreement about this change. This reverts commit 7b6c114.
abhinav
added a commit
that referenced
this pull request
Oct 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enable the paralleltest linter.
This linter checks that all tests and subtests call t.Parallel.
Fix all fixable issues reported by the linter.
For cases where a test cannot be run in parallel,
add a
//nolint:paralleltestcomment with an appropriate explanation.For example:
Best reviewed by ignoring all whitespace changes.
Full list fixed issues