Split tools_tests subshards into separate shards to support sub-sub-sharding (#75033)

dev/bots/test.dart

@@ -62,6 +62,9 @@ final bool useFlutterTestFormatter = Platform.environment['FLUTTER_TEST_FORMATTE
 /// and make sure it runs _all_ shards.
 const int kBuildTestShardCount = 2;
 
+const String kShardKey = 'SHARD';
+const String kSubshardKey = 'SUBSHARD';
+
 /// The number of Cirrus jobs that run Web tests in parallel.
 ///
 /// The default is 8 shards. Typically .cirrus.yml would define the
@@ -88,6 +91,8 @@ const List<String> kWebTestFileKnownFailures = <String>[
   'test/examples/sector_layout_test.dart',
 ];
 
+const String kSmokeTestShardName = 'smoke_tests';
+
 /// When you call this, you can pass additional arguments to pass custom
 /// arguments to flutter test. For example, you might want to call this
 /// script with the parameter --local-engine=host_debug_unopt to
@@ -107,18 +112,22 @@ Future<void> main(List<String> args) async {
     print('═' * 80);
     await _runSmokeTests();
     print('═' * 80);
-    await selectShard(const <String, ShardRunner>{
+    await selectShard(<String, ShardRunner>{
       'add_to_app_life_cycle_tests': _runAddToAppLifeCycleTests,
       'build_tests': _runBuildTests,
       'framework_coverage': _runFrameworkCoverage,
       'framework_tests': _runFrameworkTests,
       'tool_coverage': _runToolCoverage,
       'tool_tests': _runToolTests,
+      'tool_general_tests': _runGeneralToolTests,
+      'tool_command_tests': _runCommandsToolTests,
+      'tool_integration_tests': _runIntegrationToolTests,
       'web_tool_tests': _runWebToolTests,
       'web_tests': _runWebUnitTests,
       'web_integration_tests': _runWebIntegrationTests,
       'web_long_running_tests': _runWebLongRunningTests,
       'flutter_plugins': _runFlutterPluginsTests,
+      kSmokeTestShardName: () async {}, // No-op, the smoke tests already ran. Used for testing this script.
     });
   } on ExitException catch (error) {
     error.apply();
@@ -183,68 +192,96 @@ Future<void> _runSmokeTests() async {
   // We run the "pass" and "fail" smoke tests first, and alone, because those
   // are particularly critical and sensitive. If one of these fails, there's no
   // point even trying the others.
-  await _runFlutterTest(automatedTests,
+  final List<ShardRunner> tests = <ShardRunner>[
+    () => _runFlutterTest(
+          automatedTests,
           script: path.join('test_smoke_test', 'pass_test.dart'),
           printOutput: false,
-  );
-  await _runFlutterTest(automatedTests,
+        ),
+    () => _runFlutterTest(
+          automatedTests,
           script: path.join('test_smoke_test', 'fail_test.dart'),
           expectFailure: true,
           printOutput: false,
-  );
+        ),
     // We run the timeout tests individually because they are timing-sensitive.
-  await _runFlutterTest(automatedTests,
+    () => _runFlutterTest(
+          automatedTests,
           script: path.join('test_smoke_test', 'timeout_pass_test.dart'),
           expectFailure: false,
           printOutput: false,
-  );
-  await _runFlutterTest(automatedTests,
+        ),
+    () => _runFlutterTest(
+          automatedTests,
           script: path.join('test_smoke_test', 'timeout_fail_test.dart'),
           expectFailure: true,
           printOutput: false,
-  );
-  await _runFlutterTest(automatedTests,
-    script: path.join('test_smoke_test', 'pending_timer_fail_test.dart'),
+        ),
+    () => _runFlutterTest(automatedTests,
+            script:
+                path.join('test_smoke_test', 'pending_timer_fail_test.dart'),
             expectFailure: true,
-    printOutput: false,
-    outputChecker: (CommandResult result) {
+            printOutput: false, outputChecker: (CommandResult result) {
           return result.flattenedStdout.contains('failingPendingTimerTest')
               ? null
               : 'Failed to find the stack trace for the pending Timer.';
-    }
-  );
+        }),
     // We run the remaining smoketests in parallel, because they each take some
     // time to run (e.g. compiling), so we don't want to run them in series,
     // especially on 20-core machines...
-  await Future.wait<void>(
+    () => Future.wait<void>(
           <Future<void>>[
-      _runFlutterTest(automatedTests,
+            _runFlutterTest(
+              automatedTests,
               script: path.join('test_smoke_test', 'crash1_test.dart'),
               expectFailure: true,
               printOutput: false,
             ),
-      _runFlutterTest(automatedTests,
+            _runFlutterTest(
+              automatedTests,
               script: path.join('test_smoke_test', 'crash2_test.dart'),
               expectFailure: true,
               printOutput: false,
             ),
-      _runFlutterTest(automatedTests,
-        script: path.join('test_smoke_test', 'syntax_error_test.broken_dart'),
+            _runFlutterTest(
+              automatedTests,
+              script:
+                  path.join('test_smoke_test', 'syntax_error_test.broken_dart'),
               expectFailure: true,
               printOutput: false,
             ),
-      _runFlutterTest(automatedTests,
-        script: path.join('test_smoke_test', 'missing_import_test.broken_dart'),
+            _runFlutterTest(
+              automatedTests,
+              script: path.join(
+                  'test_smoke_test', 'missing_import_test.broken_dart'),
               expectFailure: true,
               printOutput: false,
             ),
-      _runFlutterTest(automatedTests,
-        script: path.join('test_smoke_test', 'disallow_error_reporter_modification_test.dart'),
+            _runFlutterTest(
+              automatedTests,
+              script: path.join('test_smoke_test',
+                  'disallow_error_reporter_modification_test.dart'),
               expectFailure: true,
               printOutput: false,
             ),
           ],
-  );
+        ),
+  ];
+
+  List<ShardRunner> testsToRun;
+
+  // Smoke tests are special and run first for all test shards.
+  // Run all smoke tests for other shards.
+  // Only shard smoke tests when explicitly specified.
+  final String shardName = Platform.environment[kShardKey];
+  if (shardName == kSmokeTestShardName) {
+    testsToRun = _selectIndexOfTotalSubshard<ShardRunner>(tests);
+  } else {
+    testsToRun = tests;
+  }
+  for (final ShardRunner test in testsToRun) {
+    await test();
+  }
 
   // Verify that we correctly generated the version file.
   final String versionError = await verifyVersion(File(path.join(flutterRoot, 'version')));
@@ -276,6 +313,42 @@ Future<void> _runToolCoverage() async {
   );
 }
 
+Future<void> _runGeneralToolTests() async {
+  await _pubRunTest(
+    path.join(flutterRoot, 'packages', 'flutter_tools'),
+    testPaths: <String>[path.join('test', 'general.shard')],
+    enableFlutterToolAsserts: false,
+    // Detect unit test time regressions (poor time delay handling, etc).
+    perTestTimeout: const Duration(seconds: 2),
+  );
+}
+
+Future<void> _runCommandsToolTests() async {
+  // Due to https://github.com/flutter/flutter/issues/46180, skip the hermetic directory
+  // on Windows.
+  final String suffix = Platform.isWindows ? 'permeable' : '';
+  await _pubRunTest(
+    path.join(flutterRoot, 'packages', 'flutter_tools'),
+    forceSingleCore: true,
+    testPaths: <String>[path.join('test', 'commands.shard', suffix)],
+  );
+}
+
+Future<void> _runIntegrationToolTests() async {
+  final String toolsPath = path.join(flutterRoot, 'packages', 'flutter_tools');
+  final List<String> allTests = Directory(path.join(toolsPath, 'test', 'integration.shard'))
+      .listSync(recursive: true).whereType<File>()
+      .map<String>((FileSystemEntity entry) => path.relative(entry.path, from: toolsPath))
+      .where((String testPath) => path.basename(testPath).endsWith('_test.dart')).toList();
+
+  await _pubRunTest(
+    toolsPath,
+    forceSingleCore: true,
+    testPaths: _selectIndexOfTotalSubshard<String>(allTests),
+  );
+}
+
+// TODO(jmagman): Remove once LUCI configs are migrated to tool_tests_general, tool_tests_command, and tool_tests_integration.
 Future<void> _runToolTests() async {
   const String kDotShard = '.shard';
   const String kWeb = 'web';
@@ -810,7 +883,10 @@ Future<void> _runWebLongRunningTests() async {
     () => _runGalleryE2eWebTest('release', canvasKit: true),
   ];
   await _ensureChromeDriverIsRunning();
+  if (!await _runShardRunnerIndexOfTotalSubshard(tests)) {
+    // TODO(jmagman): Remove fallback once LUCI configs are migrated to d+_d+ subshard format.
     await _selectIndexedSubshard(tests, kWebLongRunningTestShardCount);
+  }
   await _stopChromeDriver();
 }
 
@@ -1457,6 +1533,60 @@ Future<void> _selectIndexedSubshard(List<ShardRunner> tests, int numberOfShards)
   await selectSubshard(subshards);
 }
 
+/// Parse (one-)index/total-named subshards from environment variable SUBSHARD
+/// and equally distribute [tests] between them.
+/// Subshard format is "{index}_{total number of shards}".
+/// The scheduler can change the number of total shards without needing an additional
+/// commit in this repository.
+///
+/// Examples:
+/// 1_3
+/// 2_3
+/// 3_3
+List<T> _selectIndexOfTotalSubshard<T>(List<T> tests, {String subshardKey = kSubshardKey}) {
+  // Example: "1_3" means the first (one-indexed) shard of three total shards.
+  final String subshardName = Platform.environment[subshardKey];
+  if (subshardName == null) {
+    print('$kSubshardKey environment variable is missing, skipping sharding');
+    return tests;
+  }
+  print('$bold$subshardKey=$subshardName$reset');
+
+  final RegExp pattern = RegExp(r'^(\d+)_(\d+)$');
+  final Match match = pattern.firstMatch(subshardName);
+  if (match == null || match.groupCount != 2) {
+    print('${red}Invalid subshard name "$subshardName". Expected format "[int]_[int]" ex. "1_3"');
+    // TODO(jmagman): exit(1) here instead once LUCI configs are migrated to d+_d+ subshard format.
+    return null;
+  }
+  // One-indexed.
+  final int index = int.parse(match.group(1));
+  final int total = int.parse(match.group(2));
+  if (index > total) {
+    print('${red}Invalid subshard name "$subshardName". Index number must be greater or equal to total.');
+    exit(1);
+  }
+
+  final int testsPerShard = tests.length ~/ total;
+  final int start = (index - 1) * testsPerShard;
+  final int end = index * testsPerShard;
+
+  print('Selecting subshard $index of $total (range ${start + 1}-$end of ${tests.length})');
+  return tests.sublist(start, end);
+}
+
+Future<bool> _runShardRunnerIndexOfTotalSubshard(List<ShardRunner> tests) async {
+  final List<ShardRunner> sublist = _selectIndexOfTotalSubshard<ShardRunner>(tests);
+  // TODO(jmagman): Remove the boolean return to indicate fallback to unsharded variant once LUCI configs are migrated to d+_d+ subshard format.
+  if (sublist == null) {
+    return false;
+  }
+  for (final ShardRunner test in sublist) {
+    await test();
+  }
+  return true;
+}
+
 /// If the CIRRUS_TASK_NAME environment variable exists, we use that to determine
 /// the shard and sub-shard (parsing it in the form shard-subshard-platform, ignoring
 /// the platform).
@@ -1465,8 +1595,8 @@ Future<void> _selectIndexedSubshard(List<ShardRunner> tests, int numberOfShards)
 /// environment variables. For example, to run all the framework tests you can
 /// just set SHARD=framework_tests. To run specifically the third subshard of
 /// the Web tests you can set SHARD=web_tests SUBSHARD=2 (it's zero-based).
-Future<void> selectShard(Map<String, ShardRunner> shards) => _runFromList(shards, 'SHARD', 'shard', 0);
-Future<void> selectSubshard(Map<String, ShardRunner> subshards) => _runFromList(subshards, 'SUBSHARD', 'subshard', 1);
+Future<void> selectShard(Map<String, ShardRunner> shards) => _runFromList(shards, kShardKey, 'shard', 0);
+Future<void> selectSubshard(Map<String, ShardRunner> subshards) => _runFromList(subshards, kSubshardKey, 'subshard', 1);
 
 const String CIRRUS_TASK_NAME = 'CIRRUS_TASK_NAME';
 

dev/bots/test/test_test.dart

@@ -8,6 +8,7 @@ import 'package:file/file.dart' as fs;
 import 'package:file/memory.dart';
 import 'package:mockito/mockito.dart';
 import 'package:path/path.dart' as path;
+import 'package:process/process.dart';
 
 import '../test.dart';
 import 'common.dart';
@@ -80,4 +81,44 @@ void main() {
       expect(actualHash, kSampleHash);
     });
   });
+
+  group('test.dart script', () {
+    const ProcessManager processManager = LocalProcessManager();
+
+    Future<ProcessResult> runScript(
+        [Map<String, String> environment, List<String> otherArgs = const <String>[]]) async {
+      final String dart = path.absolute(
+          path.join('..', '..', 'bin', 'cache', 'dart-sdk', 'bin', 'dart'));
+      final ProcessResult scriptProcess = processManager.runSync(<String>[
+        dart,
+        'test.dart',
+        ...otherArgs,
+      ], environment: environment);
+      return scriptProcess;
+    }
+
+    test('subshards tests correctly', () async {
+      ProcessResult result = await runScript(
+        <String, String>{'SHARD': 'smoke_tests', 'SUBSHARD': '1_3'},
+      );
+      expect(result.exitCode, 0);
+      // There are currently 6 smoke tests. This shard should contain test 1 and 2.
+      expect(result.stdout, contains('Selecting subshard 1 of 3 (range 1-2 of 6)'));
+
+      result = await runScript(
+        <String, String>{'SHARD': 'smoke_tests', 'SUBSHARD': '5_6'},
+      );
+      expect(result.exitCode, 0);
+      // This shard should contain only test 5.
+      expect(result.stdout, contains('Selecting subshard 5 of 6 (range 5-5 of 6)'));
+    });
+
+    test('exits with code 1 when SUBSHARD index greater than total', () async {
+      final ProcessResult result = await runScript(
+        <String, String>{'SHARD': 'smoke_tests', 'SUBSHARD': '100_99'},
+      );
+      expect(result.exitCode, 1);
+      expect(result.stdout, contains('Invalid subshard name'));
+    });
+  });
 }