fix flakey tests (#35)

* fix flakey tests

* lint

* protect against race and flakey tests going forward

Author: GrantBirki

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 {

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,24 +346,33 @@ 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
 	origCombineBranchName := combineBranchName
 	origBranchPrefix := branchPrefix
 	origBranchSuffix := branchSuffix
 	origBranchRegex := branchRegex
+	mutex.Unlock()
 
 	// Restore original values after test
 	defer func() {
+		mutex.Lock()
 		ignoreLabels = origIgnoreLabels
 		selectLabels = origSelectLabels
 		caseSensitiveLabels = origCaseSensitiveLabels
 		combineBranchName = origCombineBranchName
 		branchPrefix = origBranchPrefix
 		branchSuffix = origBranchSuffix
 		branchRegex = origBranchRegex
+		mutex.Unlock()
 	}()
 
 	// Test cases

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

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