fix test sharding (#156768)

Also cleans up https://github.com/flutter/flutter/issues/156762

<details>

<summary> Pre-launch checklist </summary> 

</details>

.ci.yaml

@@ -5361,7 +5361,6 @@ targets:
       - .ci.yaml
 
   - name: Windows build_tests_1_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5380,7 +5379,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_2_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5399,7 +5397,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_3_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5418,7 +5415,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_4_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5437,7 +5433,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_5_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5456,7 +5451,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_6_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5475,7 +5469,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_7_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:
@@ -5494,7 +5487,6 @@ targets:
         ["framework", "hostonly", "shard", "windows"]
 
   - name: Windows build_tests_8_8
-    bringup: true
     recipe: flutter/flutter_drone
     timeout: 60
     properties:

dev/bots/test/test_test.dart

@@ -4,6 +4,7 @@
 
 import 'dart:io' hide Platform;
 
+import 'package:collection/collection.dart';
 import 'package:file/file.dart' as fs;
 import 'package:file/memory.dart';
 import 'package:path/path.dart' as path;
@@ -154,4 +155,60 @@ void main() {
       expect(result.stdout, contains('Invalid subshard name'));
     });
   });
+
+  test('selectTestsForSubShard distributes tests amongst subshards correctly', () async {
+    List<int> makeTests(int count) => List<int>.generate(count, (int index) => index);
+
+    void testSubsharding(int testCount, int subshardCount) {
+      String failureReason(String reason) {
+        return 'Subsharding test failed for testCount=$testCount, subshardCount=$subshardCount.\n'
+          '$reason';
+      }
+
+      final List<int> tests = makeTests(testCount);
+      final List<List<int>> subshards = List<List<int>>.generate(subshardCount, (int index) {
+        final int subShardIndex = index + 1;
+        final (int start, int end) = selectTestsForSubShard(
+          testCount: tests.length,
+          subShardIndex: subShardIndex,
+          subShardCount: subshardCount,
+        );
+        return tests.sublist(start, end);
+      });
+
+      final List<int> testedTests = subshards.flattened.toList();
+      final Set<int> deduped = Set<int>.from(subshards.flattened);
+      expect(
+        testedTests,
+        hasLength(deduped.length),
+        reason: failureReason('Subshards may have had duplicate tests.'),
+      );
+      expect(
+        testedTests,
+        unorderedEquals(tests),
+        reason: failureReason('One or more tests were not assigned to a subshard.'),
+      );
+
+      final int minimumTestsPerShard = (testCount / subshardCount).floor();
+      for (int i = 0; i < subshards.length; i++) {
+        final int extraTestsInThisShard = subshards[i].length - minimumTestsPerShard;
+        expect(
+          extraTestsInThisShard,
+          isNonNegative,
+          reason: failureReason(
+              'Subsharding uneven. Subshard ${i + 1} had too few tests: ${subshards[i].length}'),
+        );
+        expect(
+          extraTestsInThisShard,
+          lessThanOrEqualTo(1),
+          reason: failureReason(
+              'Subsharding uneven. Subshard ${i + 1} had too many tests: ${subshards[i].length}'),
+        );
+      }
+    }
+
+    testSubsharding(9, 3);
+    testSubsharding(25, 8);
+    testSubsharding(30, 15);
+  });
 }

dev/bots/utils.dart

@@ -11,6 +11,7 @@ import 'dart:math' as math;
 
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:collection/collection.dart';
 import 'package:file/file.dart' as fs;
 import 'package:file/local.dart';
 import 'package:meta/meta.dart';
@@ -534,7 +535,7 @@ Future<void> runShardRunnerIndexOfTotalSubshard(List<ShardRunner> tests) async {
 }
 /// 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 format of SUBSHARD is "{index}_{total number of shards}".
 /// The scheduler can change the number of total shards without needing an additional
 /// commit in this repository.
 ///
@@ -569,14 +570,48 @@ List<T> selectIndexOfTotalSubshard<T>(List<T> tests, {String subshardKey = kSubs
     return <T>[];
   }
 
-  final int testsPerShard = (tests.length / total).ceil();
-  final int start = (index - 1) * testsPerShard;
-  final int end = math.min(index * testsPerShard, tests.length);
-
+  final (int start, int end) = selectTestsForSubShard(
+    testCount: tests.length,
+    subShardIndex: index,
+    subShardCount: total,
+  );
   print('Selecting subshard $index of $total (tests ${start + 1}-$end of ${tests.length})');
   return tests.sublist(start, end);
 }
 
+/// Finds the interval of tests that a subshard is responsible for testing.
+@visibleForTesting
+(int start, int end) selectTestsForSubShard({
+  required int testCount,
+  required int subShardIndex,
+  required int subShardCount,
+}) {
+  // While there exists a closed formula figuring out the range of tests the
+  // subshard is resposible for, modeling this as a simulation of distributing
+  // items equally into buckets is more intuitive.
+  //
+  // A bucket represents how many tests a subshard should be allocated.
+  final List<int> buckets = List<int>.filled(subShardCount, 0);
+  // First, allocate an equal number of items to each bucket.
+  for (int i = 0; i < buckets.length; i++) {
+    buckets[i] = (testCount / subShardCount).floor();
+  }
+  // For the N leftover items, put one into each of the first N buckets.
+  final int remainingItems = testCount % buckets.length;
+  for (int i = 0; i < remainingItems; i++) {
+    buckets[i] += 1;
+  }
+
+  // Lastly, compute the indices of the items in buckets[index].
+  // We derive this from the toal number items in previous buckets and the number
+  // of items in this bucket.
+  final int numberOfItemsInPreviousBuckets = subShardIndex == 0 ? 0 : buckets.sublist(0, subShardIndex - 1).sum;
+  final int start = numberOfItemsInPreviousBuckets;
+  final int end = start + buckets[subShardIndex - 1];
+
+  return (start, end);
+}
+
 Future<void> _runFromList(Map<String, ShardRunner> items, String key, String name, int positionInTaskName) async {
   String? item = Platform.environment[key];
   if (item == null && Platform.environment.containsKey(CIRRUS_TASK_NAME)) {