diff --git a/internal/cmd/match_criteria.go b/internal/cmd/match_criteria.go index d039d79..32691b2 100644 --- a/internal/cmd/match_criteria.go +++ b/internal/cmd/match_criteria.go @@ -15,7 +15,7 @@ import ( // checks if a PR matches all filtering criteria func PrMatchesCriteria(branch string, prLabels []string) bool { // Check branch criteria if any are specified - if !branchMatchesCriteria(branch) { + if !branchMatchesCriteria(branch, combineBranchName, branchPrefix, branchSuffix, branchRegex) { return false } @@ -28,7 +28,7 @@ func PrMatchesCriteria(branch string, prLabels []string) bool { } // checks if a branch matches the branch filtering criteria -func branchMatchesCriteria(branch string) bool { +func branchMatchesCriteria(branch, combineBranchName, branchPrefix, branchSuffix, branchRegex string) bool { Logger.Debug("Checking branch criteria", "branch", branch) // Do not attempt to match on existing branches that were created by this CLI if branch == combineBranchName { diff --git a/internal/cmd/match_criteria_test.go b/internal/cmd/match_criteria_test.go index ea6de89..fc10b08 100644 --- a/internal/cmd/match_criteria_test.go +++ b/internal/cmd/match_criteria_test.go @@ -1,12 +1,11 @@ package cmd import ( + "sync" "testing" ) func TestLabelsMatch(t *testing.T) { - t.Parallel() - tests := []struct { name string prLabels []string @@ -147,19 +146,10 @@ func TestLabelsMatch(t *testing.T) { caseSensitive: true, }, } - for _, test := range tests { t.Run(test.name, func(t *testing.T) { - t.Parallel() - // Save the original value of caseSensitiveLabels - originalCaseSensitive := caseSensitiveLabels - defer func() { caseSensitiveLabels = originalCaseSensitive }() // Restore after test - - // Set caseSensitiveLabels for this test - caseSensitiveLabels = test.caseSensitive - - // Run the function + // Run the function, passing the caseSensitive parameter directly got := labelsMatch(test.prLabels, test.ignoreLabels, test.selectLabels, test.caseSensitive) if got != test.want { t.Errorf("Test %q failed: want %v, got %v", test.name, test.want, got) @@ -168,8 +158,6 @@ func TestLabelsMatch(t *testing.T) { } } func TestBranchMatchesCriteria(t *testing.T) { - // Remove t.Parallel() at the function level to avoid races on global variables - // Define test cases tests := []struct { name string @@ -269,28 +257,8 @@ func TestBranchMatchesCriteria(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() // Parallelize at the subtest level, each with their own local variables - // Save original values of global variables - origCombineBranchName := combineBranchName - origBranchPrefix := branchPrefix - origBranchSuffix := branchSuffix - origBranchRegex := branchRegex - - // Restore original values after test - defer func() { - combineBranchName = origCombineBranchName - branchPrefix = origBranchPrefix - branchSuffix = origBranchSuffix - branchRegex = origBranchRegex - }() - - // Set global variables for this specific test - combineBranchName = test.combineBranch - branchPrefix = test.prefix - branchSuffix = test.suffix - branchRegex = test.regex - // Run the function - got := branchMatchesCriteria(test.branch) + got := branchMatchesCriteria(test.branch, test.combineBranch, test.prefix, test.suffix, test.regex) // Check the result if got != test.want { @@ -378,7 +346,13 @@ func TestPrMatchesCriteriaWithMocks(t *testing.T) { } func TestPrMatchesCriteria(t *testing.T) { + // Since this test changes global state, don't use t.Parallel() + + // Create mutex to protect access to global variables + var mutex sync.Mutex + // Save original values of global variables + mutex.Lock() origIgnoreLabels := ignoreLabels origSelectLabels := selectLabels origCaseSensitiveLabels := caseSensitiveLabels @@ -386,9 +360,11 @@ func TestPrMatchesCriteria(t *testing.T) { origBranchPrefix := branchPrefix origBranchSuffix := branchSuffix origBranchRegex := branchRegex + mutex.Unlock() // Restore original values after test defer func() { + mutex.Lock() ignoreLabels = origIgnoreLabels selectLabels = origSelectLabels caseSensitiveLabels = origCaseSensitiveLabels @@ -396,6 +372,7 @@ func TestPrMatchesCriteria(t *testing.T) { branchPrefix = origBranchPrefix branchSuffix = origBranchSuffix branchRegex = origBranchRegex + mutex.Unlock() }() // Test cases diff --git a/internal/cmd/repos_test.go b/internal/cmd/repos_test.go index 8fbc2de..630ae28 100644 --- a/internal/cmd/repos_test.go +++ b/internal/cmd/repos_test.go @@ -71,7 +71,6 @@ func TestParseRepositoriesArgs(t *testing.T) { } func TestParseRepositoriesFile(t *testing.T) { - t.Parallel() tests := []struct { content string want []github.Repo diff --git a/script/test b/script/test index 5d135f0..b8ad30b 100755 --- a/script/test +++ b/script/test @@ -2,9 +2,11 @@ set -e +count=10 + # if the tparse binary is not found, don't use it if ! command -v tparse &> /dev/null; then - go test -v -cover -coverprofile=coverage.out ./... + go test -race -count $count -v -cover -coverprofile=coverage.out ./... else - set -o pipefail && go test -cover -coverprofile=coverage.out -json ./... | tparse -smallscreen -all -trimpath github.com/github/ + set -o pipefail && go test -race -count $count -cover -coverprofile=coverage.out -json ./... | tparse -smallscreen -all -trimpath github.com/github/ fi