diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 7e369fa86..377fb733d 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -11,6 +11,7 @@ all tests with OS `'windows'` would previously still run browser tests on windows, but will now skip all tests including browser tests. * Use a DevTools URL instead of a defunct observatory URL. +* Add flag `--shard-by` to control sharding strategy. ## 1.31.1 diff --git a/pkgs/test/README.md b/pkgs/test/README.md index 9da43d2a7..03ab04969 100644 --- a/pkgs/test/README.md +++ b/pkgs/test/README.md @@ -204,12 +204,35 @@ dart test --total-shards 3 --shard-index 0 path/to/test.dart dart test --total-shards 3 --shard-index 1 path/to/test.dart dart test --total-shards 3 --shard-index 2 path/to/test.dart ``` -Sharding: This refers to the process of splitting up a large test suite into -smaller subsets (called shards) that can be run independently. Sharding is -particularly useful for distributed testing, where multiple machines are used -to run tests simultaneously. By dividing the test suite into smaller subsets, -you can run tests in parallel across multiple machines, which can significantly -reduce the overall testing time. + +By default, sharding is done by `"test"`, meaning individual tests within each +suite are distributed across the shards. You can customize how tests are +distributed using the `--shard-by` flag, which supports the following values: + +* `test` (default): Distribute individual test cases across shards. This + balances the test load at the test case level. Test cases from each suite + are sliced continuously, which minimizes how often a suite is split across + shards and helps maximize the re-use of `setUpAll` and `tearDownAll` setups. +* `file`: Distribute entire test files across shards. This can be faster for + suites with many small files as it avoids loading every file in every shard. + Because test files are not split, any `setUpAll` and `tearDownAll` setups in + a file are guaranteed to run only once (on the shard running that file). + +Sharding is particularly useful for distributed testing, where multiple +machines are used to run tests simultaneously. By dividing the test suite into +smaller subsets, you can run tests in parallel across multiple machines, which +can significantly reduce the overall testing time. + +### Interaction with Test Filters + +The sharding modes interact differently with filters like `--name` or `--tags`: + +* When sharding by `test`, the sharding partition is calculated *after* + applying filters. This guarantees that the matching tests are distributed + as evenly as possible across all shards. +* When sharding by `file`, the files are partitioned *before* they are loaded + and filtered. If a filter only matches tests in a few files, some shards + might run no tests because those files were allocated to other shards. ### Test concurrency diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index 8a8594f81..c8d573b5a 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/pkgs/test/test/runner/runner_test.dart @@ -84,6 +84,8 @@ $_runtimeCompilers (defaults to "$_defaultConcurrency") --total-shards The total number of invocations of the test runner being run. --shard-index The index of this test runner invocation (of --total-shards). + --shard-by How to distribute tests across shards. + [test (default), file] --timeout The default test timeout. For example: 15s, 2x, none (defaults to "30s") --suite-load-timeout The timeout for loading a test suite. Loading the test suite includes compiling the test suite. For example: 15s, 2m, none diff --git a/pkgs/test/test/runner/shard_test.dart b/pkgs/test/test/runner/shard_test.dart index b255da9c8..01539fe37 100644 --- a/pkgs/test/test/runner/shard_test.dart +++ b/pkgs/test/test/runner/shard_test.dart @@ -172,6 +172,147 @@ void main() { await test.shouldExit(79); }); + test('shards by file', () async { + await d.file('1_test.dart', ''' + import 'package:test/test.dart'; + + void main() { + test("test 1.1", () {}); + test("test 1.2", () {}); + } + ''').create(); + + await d.file('2_test.dart', ''' + import 'package:test/test.dart'; + + void main() { + test("test 2.1", () {}); + test("test 2.2", () {}); + } + ''').create(); + + var test = await runTest([ + '.', + '--shard-index=0', + '--total-shards=2', + '--shard-by=file', + ]); + expect( + test.stdout, + containsInOrder([ + '+0: ./1_test.dart: test 1.1', + '+1: ./1_test.dart: test 1.2', + '+2: All tests passed!', + ]), + ); + expect(test.stdout, isNot(contains('./2_test.dart'))); + await test.shouldExit(0); + + test = await runTest([ + '.', + '--shard-index=1', + '--total-shards=2', + '--shard-by=file', + ]); + expect( + test.stdout, + containsInOrder([ + '+0: ./2_test.dart: test 2.1', + '+1: ./2_test.dart: test 2.2', + '+2: All tests passed!', + ]), + ); + expect(test.stdout, isNot(contains('./1_test.dart'))); + await test.shouldExit(0); + }); + + test('interaction of --shard-by=file with name filters', () async { + await d.file('1_test.dart', ''' + import 'package:test/test.dart'; + + void main() { + test("match", () {}); + } + ''').create(); + + await d.file('2_test.dart', ''' + import 'package:test/test.dart'; + + void main() { + test("other", () {}); + } + ''').create(); + + // Shard 1 gets 2_test.dart. But 2_test.dart has no tests matching 'match'. + // So shard 1 should report no tests ran and fail. + var test = await runTest([ + '.', + '--shard-index=1', + '--total-shards=2', + '--shard-by=file', + '--name=match', + ]); + expect(test.stdout, emitsThrough('No tests ran.')); + await test.shouldExit(79); + + // Shard 0 gets 1_test.dart. It matches, so it runs. + test = await runTest([ + '.', + '--shard-index=0', + '--total-shards=2', + '--shard-by=file', + '--name=match', + ]); + expect( + test.stdout, + containsInOrder(['+0: ./1_test.dart: match', '+1: All tests passed!']), + ); + await test.shouldExit(0); + }); + + test('interaction of --shard-by=test with name filters', () async { + await d.file('test.dart', ''' + import 'package:test/test.dart'; + + void main() { + test("match 1", () {}); + test("match 2", () {}); + test("other 1", () {}); + test("other 2", () {}); + } + ''').create(); + + // Only tests matching 'match' are sharded. 'match 1' and 'match 2' are active. + // Shard 0 runs 'match 1', Shard 1 runs 'match 2'. + var test = await runTest([ + 'test.dart', + '--shard-index=0', + '--total-shards=2', + '--shard-by=test', + '--name=match', + ]); + expect( + test.stdout, + containsInOrder(['+0: match 1', '+1: All tests passed!']), + ); + expect(test.stdout, isNot(contains('match 2'))); + await test.shouldExit(0); + + test = await runTest([ + 'test.dart', + '--shard-index=1', + '--total-shards=2', + '--shard-by=test', + '--name=match', + ]); + expect( + test.stdout, + containsInOrder(['+0: match 2', '+1: All tests passed!']), + ); + expect(test.stdout, isNot(contains('match 1'))); + await test.shouldExit(0); + }); + group('reports an error if', () { test('--shard-index is provided alone', () async { var test = await runTest(['--shard-index=1']); diff --git a/pkgs/test/test/utils.dart b/pkgs/test/test/utils.dart index 26dbd06c9..e3fdd055d 100644 --- a/pkgs/test/test/utils.dart +++ b/pkgs/test/test/utils.dart @@ -232,6 +232,7 @@ Configuration configuration({ int? concurrency, int? shardIndex, int? totalShards, + String? shardBy, Map>? testSelections, Iterable? foldTraceExcept, Iterable? foldTraceOnly, @@ -286,6 +287,7 @@ Configuration configuration({ concurrency: concurrency, shardIndex: shardIndex, totalShards: totalShards, + shardBy: shardBy, testSelections: testSelections, foldTraceExcept: foldTraceExcept, foldTraceOnly: foldTraceOnly, diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index 6907873b9..d419b4424 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -2,6 +2,7 @@ * Support using the OS platform selector to configure browser tests. * Use a DevTools URL instead of a defunct observatory URL. +* Add flag `--shard-by` to control sharding strategy. ## 0.6.18 diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart index 455622f81..18dc7cad3 100644 --- a/pkgs/test_core/lib/src/runner.dart +++ b/pkgs/test_core/lib/src/runner.dart @@ -7,6 +7,7 @@ import 'dart:io'; import 'package:async/async.dart'; import 'package:boolean_selector/boolean_selector.dart'; +import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart'; import 'package:test_api/backend.dart' show PlatformSelector, Runtime, SuitePlatform; @@ -269,25 +270,76 @@ class Runner { /// Only tests that match [_config.patterns] will be included in the /// suites once they're loaded. Stream _loadSuites() { - return StreamGroup.merge( - _config.testSelections.entries.map((pathEntry) { + final Stream source; + if (_config.totalShards != null && _config.shardBy == 'file') { + final allFiles = []; + for (var pathEntry in _config.testSelections.entries) { final testPath = pathEntry.key; - final testSelections = pathEntry.value; - final suiteConfig = _config.suiteDefaults.selectTests(testSelections); if (Directory(testPath).existsSync()) { - return _loader.loadDir(testPath, suiteConfig); + allFiles.addAll( + Directory(testPath) + .listSync(recursive: true) + .whereType() + .map((f) => f.path) + .where((path) => _config.filename.matches(p.basename(path))), + ); } else if (File(testPath).existsSync()) { - return _loader.loadFile(testPath, suiteConfig); + allFiles.add(testPath); } else { - return Stream.fromIterable([ - LoadSuite.forLoadException( - LoadException(testPath, 'Does not exist.'), - suiteConfig, - ), - ]); + allFiles.add(testPath); } - }), - ).map((loadSuite) { + } + + allFiles.sort(); + + final shardSize = allFiles.length / _config.totalShards!; + final shardStart = (shardSize * _config.shardIndex!).round(); + final shardEnd = (shardSize * (_config.shardIndex! + 1)).round(); + final shardFiles = allFiles.sublist(shardStart, shardEnd); + + source = StreamGroup.merge( + shardFiles.map((path) { + final key = _config.testSelections.keys.firstWhere( + (k) => path == k || p.isWithin(k, path), + orElse: () => _config.testSelections.keys.first, + ); + final testSelections = _config.testSelections[key]!; + final suiteConfig = _config.suiteDefaults.selectTests(testSelections); + + if (!File(path).existsSync() && !Directory(path).existsSync()) { + return Stream.fromIterable([ + LoadSuite.forLoadException( + LoadException(path, 'Does not exist.'), + suiteConfig, + ), + ]); + } + return _loader.loadFile(path, suiteConfig); + }), + ); + } else { + source = StreamGroup.merge( + _config.testSelections.entries.map((pathEntry) { + final testPath = pathEntry.key; + final testSelections = pathEntry.value; + final suiteConfig = _config.suiteDefaults.selectTests(testSelections); + if (Directory(testPath).existsSync()) { + return _loader.loadDir(testPath, suiteConfig); + } else if (File(testPath).existsSync()) { + return _loader.loadFile(testPath, suiteConfig); + } else { + return Stream.fromIterable([ + LoadSuite.forLoadException( + LoadException(testPath, 'Does not exist.'), + suiteConfig, + ), + ]); + } + }), + ); + } + + return source.map((loadSuite) { return loadSuite.changeSuite((suite) { _warnForUnknownTags(suite); @@ -482,15 +534,15 @@ class Runner { return 'the suite itself'; } - /// If sharding is enabled, filters [suite] to only include the tests that - /// should be run in this shard. + /// If sharding by "test" is enabled, filters [suite] to only include the + /// tests that should be run in this shard. /// /// We just take a slice of the tests in each suite corresponding to the shard - /// index. This makes the tests pretty tests across shards, and since the + /// index. This makes the tests pretty even across shards, and since the /// tests are continuous, makes us more likely to be able to re-use /// `setUpAll()` logic. RunnerSuite _shardSuite(RunnerSuite suite) { - if (_config.totalShards == null) return suite; + if (_config.totalShards == null || _config.shardBy == 'file') return suite; var shardSize = suite.group.testCount / _config.totalShards!; var shardIndex = _config.shardIndex!; diff --git a/pkgs/test_core/lib/src/runner/configuration.dart b/pkgs/test_core/lib/src/runner/configuration.dart index 3f432ada7..aedd15243 100644 --- a/pkgs/test_core/lib/src/runner/configuration.dart +++ b/pkgs/test_core/lib/src/runner/configuration.dart @@ -130,6 +130,10 @@ class Configuration { /// See [shardIndex] for details. final int? totalShards; + /// How to distribute tests across shards. + String get shardBy => _shardBy ?? 'test'; + final String? _shardBy; + /// The list of packages to fold when producing [StackTrace]s. Set get foldTraceExcept => _foldTraceExcept ?? {}; final Set? _foldTraceExcept; @@ -283,6 +287,7 @@ class Configuration { required int? concurrency, required int? shardIndex, required int? totalShards, + required String? shardBy, required Map>? testSelections, required Iterable? foldTraceExcept, required Iterable? foldTraceOnly, @@ -340,6 +345,7 @@ class Configuration { concurrency: concurrency, shardIndex: shardIndex, totalShards: totalShards, + shardBy: shardBy, testSelections: testSelections, foldTraceExcept: foldTraceExcept, foldTraceOnly: foldTraceOnly, @@ -403,6 +409,7 @@ class Configuration { int? concurrency, int? shardIndex, int? totalShards, + String? shardBy, Map>? testSelections, Iterable? foldTraceExcept, Iterable? foldTraceOnly, @@ -458,6 +465,7 @@ class Configuration { concurrency: concurrency, shardIndex: shardIndex, totalShards: totalShards, + shardBy: shardBy, testSelections: testSelections, foldTraceExcept: foldTraceExcept, foldTraceOnly: foldTraceOnly, @@ -531,6 +539,7 @@ class Configuration { concurrency: null, shardIndex: null, totalShards: null, + shardBy: null, testSelections: null, filename: null, chosenPresets: null, @@ -599,6 +608,7 @@ class Configuration { concurrency: null, shardIndex: null, totalShards: null, + shardBy: null, testSelections: null, foldTraceExcept: null, foldTraceOnly: null, @@ -669,6 +679,7 @@ class Configuration { coveragePackages: null, shardIndex: null, totalShards: null, + shardBy: null, testSelections: null, foldTraceExcept: null, foldTraceOnly: null, @@ -735,6 +746,7 @@ class Configuration { concurrency: null, shardIndex: null, totalShards: null, + shardBy: null, foldTraceExcept: null, foldTraceOnly: null, chosenPresets: null, @@ -806,6 +818,7 @@ class Configuration { required int? concurrency, required this.shardIndex, required this.totalShards, + required String? shardBy, required Map>? testSelections, required Iterable? foldTraceExcept, required Iterable? foldTraceOnly, @@ -821,7 +834,8 @@ class Configuration { required BooleanSelector? excludeTags, required Iterable? globalPatterns, required SuiteConfiguration? suiteDefaults, - }) : _help = help, + }) : _shardBy = shardBy, + _help = help, _version = version, _pauseAfterLoad = pauseAfterLoad, _debug = debug, @@ -899,6 +913,7 @@ class Configuration { concurrency: null, shardIndex: null, totalShards: null, + shardBy: null, testSelections: null, foldTraceExcept: null, foldTraceOnly: null, @@ -1002,6 +1017,7 @@ class Configuration { concurrency: other._concurrency ?? _concurrency, shardIndex: other.shardIndex ?? shardIndex, totalShards: other.totalShards ?? totalShards, + shardBy: other._shardBy ?? _shardBy, testSelections: other._testSelections ?? _testSelections, foldTraceExcept: foldTraceExcept, foldTraceOnly: foldTraceOnly, @@ -1059,6 +1075,7 @@ class Configuration { int? concurrency, int? shardIndex, int? totalShards, + String? shardBy, Map>? testSelections, Iterable? exceptPackages, Iterable? onlyPackages, @@ -1110,6 +1127,7 @@ class Configuration { concurrency: concurrency ?? _concurrency, shardIndex: shardIndex ?? this.shardIndex, totalShards: totalShards ?? this.totalShards, + shardBy: shardBy ?? _shardBy, testSelections: testSelections ?? _testSelections, foldTraceExcept: exceptPackages ?? _foldTraceExcept, foldTraceOnly: onlyPackages ?? _foldTraceOnly, diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart index 1714784f3..415409a64 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/pkgs/test_core/lib/src/runner/configuration/args.dart @@ -129,6 +129,12 @@ final ArgParser _parser = (() { 'shard-index', help: 'The index of this test runner invocation (of --total-shards).', ); + parser.addOption( + 'shard-by', + help: 'How to distribute tests across shards.', + allowed: ['test', 'file'], + defaultsTo: 'test', + ); parser.addOption( 'pub-serve', help: '[Removed] The port of a pub serve instance serving "test/".', @@ -379,6 +385,7 @@ class _Parser { var shardIndex = _parseOption('shard-index', int.parse); var totalShards = _parseOption('total-shards', int.parse); + var shardBy = _ifParsed('shard-by') as String?; if ((shardIndex == null) != (totalShards == null)) { throw const FormatException( '--shard-index and --total-shards may only be passed together.', @@ -477,6 +484,7 @@ class _Parser { concurrency: _parseOption('concurrency', int.parse), shardIndex: shardIndex, totalShards: totalShards, + shardBy: shardBy, timeout: _parseOption('timeout', Timeout.parse), suiteLoadTimeout: _parseOption('suite-load-timeout', Timeout.parse), globalPatterns: patterns,