Fix envtool run test -run and -skip flags

.golangci.yml

@@ -3,6 +3,8 @@
 
 run:
   timeout: 3m
+  skip-dirs:
+    - cmd/envtool/internal/testmatch # due to files match.go and match_test.go from the Go standard library
 
 linters-settings:
   # asciicheck

cmd/envtool/tests.go

@@ -363,21 +363,30 @@ func testsRun(ctx context.Context, index, total uint, run, skip string, args []s
 		return fmt.Errorf("--shard-index and --shard-total must be specified when --run is not")
 	}
 
-	all, err := listTestFuncsWithRegex("", run, skip)
+	tests, err := listTestFuncsWithRegex("", run, skip)
 	if err != nil {
 		return lazyerrors.Error(err)
 	}
 
-	if len(all) == 0 {
-		return fmt.Errorf("no tests to run")
-	}
-
-	shard, err := shardTestFuncs(index, total, all)
+	// Then, shard all the tests but only run the ones that match the regex and that should
+	// be run on the specific shard.
+	shard, skipShard, err := shardTestFuncs(index, total, tests)
 	if err != nil {
 		return lazyerrors.Error(err)
 	}
 
-	args = append(args, "-run="+buildGoTestRunRegex(shard))
+	args = append(args, "-run="+run)
+
+	if len(skipShard) > 0 {
+		if skip != "" {
+			skip += "|"
+		}
+		skip += "^(" + strings.Join(skipShard, "|") + ")$"
+	}
+
+	if skip != "" {
+		args = append(args, "-skip="+skip)
+	}
 
 	return runGoTest(ctx, args, len(shard), true, logger)
 }
@@ -437,100 +446,87 @@ func listTestFuncs(dir string) ([]string, error) {
 // functions (tests, benchmarks, examples, fuzz functions) in the specified
 // directory and subdirectories.
 func listTestFuncsWithRegex(dir, run, skip string) ([]string, error) {
-	tests, err := listTestFuncs(dir)
+	all, err := listTestFuncs(dir)
 	if err != nil {
 		return nil, err
 	}
 
-	if run == "" && skip == "" {
-		return tests, nil
+	if len(all) == 0 {
+		return nil, fmt.Errorf("no tests to run")
 	}
 
-	includeRegex, err := regexp.Compile(run)
-	if err != nil {
-		return nil, err
-	}
+	// Filter what top-level functions we want to test using the same logic as "go test".
+	var (
+		rxRun  *regexp.Regexp
+		rxSkip *regexp.Regexp
+	)
 
-	if skip == "" {
-		return filterStringsByRegex(tests, includeRegex, nil), nil
+	if run != "" {
+		rxRun, err = regexp.Compile(run)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	excludeRegex, err := regexp.Compile(skip)
-	if err != nil {
-		return nil, err
+	if skip != "" {
+		rxSkip, err = regexp.Compile(skip)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	return filterStringsByRegex(tests, includeRegex, excludeRegex), nil
+	return filterStringsByRegex(all, rxRun, rxSkip), nil
 }
 
 // filterStringsByRegex filters a slice of strings based on inclusion and exclusion
 // criteria defined by regular expressions.
 func filterStringsByRegex(tests []string, include, exclude *regexp.Regexp) []string {
 	res := []string{}
 
-	for _, test := range tests {
-		if exclude != nil && exclude.MatchString(test) {
-			continue
-		}
-
-		if include != nil && !include.MatchString(test) {
-			continue
+	for _, t := range tests {
+		if (exclude == nil || !exclude.MatchString(t)) &&
+			(include == nil || include.MatchString(t)) {
+			res = append(res, t)
 		}
-
-		res = append(res, test)
 	}
 
 	return res
 }
 
-// buildGoTestRunRegex builds a regex for `go test -run` from the given test names.
-func buildGoTestRunRegex(tests []string) string {
-	var sb strings.Builder
-	sb.WriteString("^(")
-
-	for i, test := range tests {
-		if i != 0 {
-			sb.WriteString("|")
-		}
-
-		sb.WriteString(test)
-	}
-
-	sb.WriteString(")$")
-
-	return sb.String()
-}
-
 // shardTestFuncs shards given top-level test functions.
-func shardTestFuncs(index, total uint, testFuncs []string) ([]string, error) {
+// It returns a slice of test functions to run and what test functions to skip for the given shard.
+func shardTestFuncs(index, total uint, testFuncs []string) (run, skip []string, err error) {
 	if index == 0 {
-		return nil, fmt.Errorf("index must be greater than 0")
+		return nil, nil, fmt.Errorf("index must be greater than 0")
 	}
 
 	if total == 0 {
-		return nil, fmt.Errorf("total must be greater than 0")
+		return nil, nil, fmt.Errorf("total must be greater than 0")
 	}
 
 	if index > total {
-		return nil, fmt.Errorf("cannot shard when index is greater than total (%d > %d)", index, total)
+		return nil, nil, fmt.Errorf("cannot shard when index is greater than total (%d > %d)", index, total)
 	}
 
 	l := uint(len(testFuncs))
 	if total > l {
-		return nil, fmt.Errorf("cannot shard when total is greater than a number of test functions (%d > %d)", total, l)
+		return nil, nil, fmt.Errorf("cannot shard when total is greater than a number of test functions (%d > %d)", total, l)
 	}
 
-	res := make([]string, 0, l/total+1)
+	run = make([]string, 0, l/total+1)
+	skip = make([]string, 0, len(testFuncs)-len(run))
 	shard := uint(1)
 
 	// use different shards for tests with similar names for better load balancing
 	for _, test := range testFuncs {
 		if index == shard {
-			res = append(res, test)
+			run = append(run, test)
+		} else {
+			skip = append(skip, test)
 		}
 
 		shard = shard%total + 1
 	}
 
-	return res, nil
+	return run, skip, nil
 }

cmd/envtool/tests_test.go

@@ -313,43 +313,6 @@ func TestListTestFuncsWithRegex(t *testing.T) {
 	}
 }
 
-func TestBuildGoTestRunRegex(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name     string
-		expected string
-		tests    []string
-	}{
-		{
-			name:     "Empty",
-			tests:    []string{},
-			expected: "^()$",
-		},
-		{
-			name:     "Single",
-			tests:    []string{"Test1"},
-			expected: "^(Test1)$",
-		},
-		{
-			name:     "Multiple",
-			tests:    []string{"Test1", "Test2"},
-			expected: "^(Test1|Test2)$",
-		},
-	}
-
-	for _, tt := range tests {
-		tt := tt
-
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			actual := buildGoTestRunRegex(tt.tests)
-			assert.Equal(t, tt.expected, actual)
-		})
-	}
-}
-
 func TestFilterStringsByRegex(t *testing.T) {
 	t.Parallel()
 
@@ -427,34 +390,35 @@ func TestShardTestFuncs(t *testing.T) {
 	t.Run("InvalidIndex", func(t *testing.T) {
 		t.Parallel()
 
-		_, err := shardTestFuncs(0, 3, testFuncs)
+		_, _, err := shardTestFuncs(0, 3, testFuncs)
 		assert.EqualError(t, err, "index must be greater than 0")
 
-		_, err = shardTestFuncs(3, 3, testFuncs)
+		_, _, err = shardTestFuncs(3, 3, testFuncs)
 		assert.NoError(t, err)
 
-		_, err = shardTestFuncs(4, 3, testFuncs)
+		_, _, err = shardTestFuncs(4, 3, testFuncs)
 		assert.EqualError(t, err, "cannot shard when index is greater than total (4 > 3)")
 	})
 
 	t.Run("InvalidTotal", func(t *testing.T) {
 		t.Parallel()
 
-		_, err := shardTestFuncs(3, 1000, testFuncs[:42])
+		_, _, err := shardTestFuncs(3, 1000, testFuncs[:42])
 		assert.EqualError(t, err, "cannot shard when total is greater than a number of test functions (1000 > 42)")
 	})
 
 	t.Run("Valid", func(t *testing.T) {
 		t.Parallel()
 
-		res, err := shardTestFuncs(1, 3, testFuncs)
+		res, skip, err := shardTestFuncs(1, 3, testFuncs)
 		require.NoError(t, err)
 		assert.Equal(t, testFuncs[0], res[0])
 		assert.NotEqual(t, testFuncs[1], res[1])
 		assert.NotEqual(t, testFuncs[2], res[1])
 		assert.Equal(t, testFuncs[3], res[1])
+		assert.NotEmpty(t, skip)
 
-		res, err = shardTestFuncs(3, 3, testFuncs)
+		res, _, err = shardTestFuncs(3, 3, testFuncs)
 		require.NoError(t, err)
 		assert.NotEqual(t, testFuncs[0], res[0])
 		assert.NotEqual(t, testFuncs[1], res[0])