Fix envtool run test -run and -skip flags (#4101)

cmd/envtool/testdata/subtest_test.go

@@ -0,0 +1,13 @@
+package testdata
+
+import "testing"
+
+func TestWithSubtest(t *testing.T) {
+	t.Run("First", func(t *testing.T) {})
+	t.Run("Second", func(t *testing.T) {})
+	t.Run("Third", func(t *testing.T) {
+		t.Run("NestedOne", func(t *testing.T) {})
+		t.Run("NestedTwo", func(t *testing.T) {})
+		t.Run("NestedThree", func(t *testing.T) {})
+	})
+}

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)
 }
@@ -442,25 +451,30 @@ func listTestFuncsWithRegex(dir, run, skip string) ([]string, error) {
 		return nil, err
 	}
 
-	if run == "" && skip == "" {
-		return tests, nil
+	if len(tests) == 0 {
+		return nil, fmt.Errorf("no tests to run")
 	}
 
-	includeRegex, err := regexp.Compile(run)
-	if err != nil {
-		return nil, err
-	}
+	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(tests, rxRun, rxSkip), nil
 }
 
 // filterStringsByRegex filters a slice of strings based on inclusion and exclusion
@@ -483,54 +497,40 @@ func filterStringsByRegex(tests []string, include, exclude *regexp.Regexp) []str
 	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

@@ -19,6 +19,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"regexp"
+	"sort"
 	"strings"
 	"testing"
 
@@ -98,6 +99,55 @@ func TestRunGoTest(t *testing.T) {
 		assert.Equal(t, expected, actual, "actual:\n%s", strings.Join(actual, "\n"))
 	})
 
+	t.Run("SubtestsPartial", func(t *testing.T) {
+		t.Parallel()
+
+		var actual []string
+		logger, err := makeTestLogger(&actual)
+		require.NoError(t, err)
+
+		err = runGoTest(context.TODO(), []string{
+			"./testdata",
+			"-count=1",
+			"-run=TestWithSubtest/Third",
+		}, 1, false, logger.Sugar())
+		require.NoError(t, err)
+
+		expected := []string{
+			"PASS TestWithSubtest 1/1",
+			"PASS",
+			"ok  	github.com/FerretDB/FerretDB/cmd/envtool/testdata	<SEC>s",
+			"PASS github.com/FerretDB/FerretDB/cmd/envtool/testdata",
+		}
+
+		cleanup(actual)
+
+		assert.Equal(t, expected, actual, "actual:\n%s", strings.Join(actual, "\n"))
+	})
+
+	t.Run("SubtestsNotFound", func(t *testing.T) {
+		t.Parallel()
+
+		var actual []string
+		logger, err := makeTestLogger(&actual)
+		require.NoError(t, err)
+
+		err = runGoTest(context.TODO(), []string{"./testdata", "-count=1", "-run=TestWithSubtest/None"}, 1, false, logger.Sugar())
+		require.NoError(t, err)
+
+		expected := []string{
+			"PASS TestWithSubtest 1/1",
+			"testing: warning: no tests to run",
+			"PASS",
+			"ok  	github.com/FerretDB/FerretDB/cmd/envtool/testdata	<SEC>s [no tests to run]",
+			"PASS github.com/FerretDB/FerretDB/cmd/envtool/testdata",
+		}
+
+		cleanup(actual)
+
+		assert.Equal(t, expected, actual, "actual:\n%s", strings.Join(actual, "\n"))
+	})
+
 	t.Run("Error", func(t *testing.T) {
 		t.Parallel()
 
@@ -218,6 +268,7 @@ func TestListTestFuncs(t *testing.T) {
 		"TestNormal2",
 		"TestPanic1",
 		"TestSkip1",
+		"TestWithSubtest",
 	}
 	assert.Equal(t, expected, actual)
 }
@@ -241,6 +292,7 @@ func TestListTestFuncsWithRegex(t *testing.T) {
 				"TestNormal2",
 				"TestPanic1",
 				"TestSkip1",
+				"TestWithSubtest",
 			},
 			wantErr: assert.NoError,
 		},
@@ -263,6 +315,7 @@ func TestListTestFuncsWithRegex(t *testing.T) {
 				"TestNormal2",
 				"TestPanic1",
 				"TestSkip1",
+				"TestWithSubtest",
 			},
 			wantErr: assert.NoError,
 		},
@@ -313,43 +366,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,37 +443,62 @@ 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)
+		lastRes, lastSkip, err := shardTestFuncs(3, 3, testFuncs)
 		require.NoError(t, err)
-		assert.NotEqual(t, testFuncs[0], res[0])
-		assert.NotEqual(t, testFuncs[1], res[0])
-		assert.Equal(t, testFuncs[2], res[0])
+		assert.NotEqual(t, testFuncs[0], lastRes[0])
+		assert.NotEqual(t, testFuncs[1], lastRes[0])
+		assert.Equal(t, testFuncs[2], lastRes[0])
+		assert.NotEmpty(t, lastSkip)
+
+		assert.NotEqual(t, res, lastRes)
+		assert.NotEqual(t, skip, lastSkip)
 	})
 }
+
+func TestListTestFuncsWithSkip(t *testing.T) {
+	t.Parallel()
+
+	testFuncs, err := listTestFuncsWithRegex(filepath.Join("testdata"), "", "Skip")
+	require.NoError(t, err)
+
+	sort.Strings(testFuncs)
+
+	res, skip, err := shardTestFuncs(1, 2, testFuncs)
+
+	assert.Equal(t, []string{"TestError2", "TestNormal2", "TestWithSubtest"}, skip)
+	assert.Equal(t, []string{"TestError1", "TestNormal1", "TestPanic1"}, res)
+	assert.Nil(t, err)
+
+	lastRes, lastSkip, err := shardTestFuncs(3, 3, testFuncs)
+	assert.Equal(t, []string{"TestNormal1", "TestWithSubtest"}, lastRes)
+	assert.Equal(t, []string{"TestError1", "TestError2", "TestNormal2", "TestPanic1"}, lastSkip)
+	require.NoError(t, err)
+}