test: enable parallel execution across test suite#57
Merged
Conversation
- Add t.Parallel() to top-level tests and relevant subtests across many packages: auth (jwt, paseto), io (copy, dir, file, remove, temp, ownership_unix), converters, encoding, memory, mfa (backup, hotp, rate_limit, totp), password (argon2id, bcrypt, compare), sanitize (filename, html, markdown, nosql/sql), tlsconfig, tokens, validate (email, url), and internal/io. - No production code changes; only *_test.go files touched. - Expected to reduce wall-clock time for `go test ./...` and encourage concurrency-safe tests.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request enables parallel test execution across the entire test suite by adding t.Parallel() calls to top-level tests and relevant subtests. The change aims to reduce wall-clock time for running the test suite and encourages writing concurrency-safe tests.
Changes:
- Added
t.Parallel()to top-level test functions across 30+ test files in packages including auth, io, converters, encoding, memory, mfa, password, sanitize, tlsconfig, tokens, and validate - Added
t.Parallel()to subtests where appropriate, ensuring parallel execution within table-driven and grouped tests - Modified cleanup patterns in
internal/io/io_test.goto uset.Cleanup()instead ofdeferfor better compatibility with parallel tests
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/validate/url_test.go | Added t.Parallel() to 7 URL validation tests |
| pkg/validate/email_test.go | Added t.Parallel() to 11 email validation tests |
| pkg/tokens/tokens_test.go | Added t.Parallel() to 6 token generation/validation tests |
| pkg/tlsconfig/config_test.go | Added t.Parallel() to 9 TLS configuration tests |
| pkg/secrets/secrets_test.go | Added t.Parallel() to 18 top-level tests and 19 subtests for secret detection and redaction |
| pkg/sanitize/sql_test.go | Added t.Parallel() to 5 SQL sanitization tests |
| pkg/sanitize/nosql_detect_test.go | Added t.Parallel() to 3 NoSQL injection detection tests |
| pkg/sanitize/markdown_test.go | Added t.Parallel() to 3 markdown sanitization tests |
| pkg/sanitize/html_test.go | Added t.Parallel() to 4 HTML sanitization tests |
| pkg/sanitize/filename_test.go | Added t.Parallel() to 5 filename sanitization tests |
| pkg/password/compare_test.go | Added t.Parallel() to constant-time comparison test |
| pkg/password/bcrypt_test.go | Added t.Parallel() to 2 bcrypt hashing tests |
| pkg/password/argon2id_test.go | Added t.Parallel() to 2 Argon2id hashing tests |
| pkg/mfa/totp_test.go | Added t.Parallel() to 6 TOTP tests and 3 subtests |
| pkg/mfa/rate_limit_test.go | Added t.Parallel() to 4 MFA rate limiting tests |
| pkg/mfa/hotp_test.go | Added t.Parallel() to 8 HOTP tests and 2 subtests |
| pkg/mfa/backup_test.go | Added t.Parallel() to 5 backup code tests and 3 subtests |
| pkg/memory/secure_buffer_test.go | Added t.Parallel() to 13 secure buffer tests |
| pkg/memory/reader_test.go | Added t.Parallel() to 5 secure buffer reader tests |
| pkg/io/temp_test.go | Added t.Parallel() to 3 temporary file/directory tests |
| pkg/io/remove_test.go | Added t.Parallel() to 4 secure file removal tests |
| pkg/io/ownership_unix_test.go | Added t.Parallel() to 3 Unix file ownership tests |
| pkg/io/file_test.go | Added t.Parallel() to 21 secure file I/O tests |
| pkg/io/dir_test.go | Added t.Parallel() to 3 directory operation tests |
| pkg/io/copy_test.go | Added t.Parallel() to 3 file copy tests |
| pkg/encoding/encoding_test.go | Added t.Parallel() to 10 encoding/decoding tests |
| pkg/converters/convert_test.go | Added t.Parallel() to 17 type conversion tests |
| pkg/auth/paseto_test.go | Added t.Parallel() to 3 PASETO token tests |
| pkg/auth/jwt_test.go | Added t.Parallel() to 3 JWT tests |
| internal/io/io_test.go | Added t.Parallel() to 2 tests and 3 subtests; replaced defer cleanup with t.Cleanup() |
Comments suppressed due to low confidence (1)
internal/io/io_test.go:102
- This test has a concurrency issue when both the parent test and subtest are marked as parallel. The parent test creates files (validPath and symlinkPath) and registers cleanup handlers. When the subtest runs in parallel, the parent test's cleanup can execute before the subtest completes, causing the symlink to be deleted while the subtest is still using it. Additionally, the filenames are not unique, which could cause conflicts when multiple parallel test runs occur.
To fix this, either:
- Remove t.Parallel() from the subtest (line 97) so it waits for the parent to complete, or
- Remove t.Parallel() from the parent test, or
- Restructure the test so the subtest doesn't depend on the parent's file resources.
func TestSecurePathSymlink(t *testing.T) {
t.Parallel()
tempDir := os.TempDir()
validPath := filepath.Join(tempDir, "valid.txt")
symlinkPath := filepath.Join(tempDir, "symlink.txt")
err := os.WriteFile(validPath, []byte("test"), 0o644)
require.NoError(t, err)
t.Cleanup(func() { _ = os.Remove(validPath) })
err = os.Symlink("/etc/hosts", symlinkPath)
if err == nil {
t.Cleanup(func() { _ = os.Remove(symlinkPath) })
t.Run("symlink outside temp directory", func(t *testing.T) {
t.Parallel()
_, err := SecurePath("symlink.txt")
assert.Error(t, err)
})
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or 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
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.
go test ./...and encourage concurrency-safe tests.