New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix envtool run test
-run
and -skip
flags
#4101
Changes from all commits
5ff7190
2d8fc80
438e31d
eda2ab3
9334c5f
a02e6ba
d9cc82c
ee288a7
7055e01
84b6eec
eafa245
b0b7a58
653adce
7e22bf0
266af4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) {}) | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what we are doing there. #3969's title is "Process What I think we should do is what the issue says. In more detail:
|
||
} | ||
|
||
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 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that
shard
's content is not actually used