diff --git a/.ci.yaml b/.ci.yaml index e95df5acbb..ff0c1a888e 100644 --- a/.ci.yaml +++ b/.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: diff --git a/dev/bots/test/test_test.dart b/dev/bots/test/test_test.dart index e620de8f3a..2350e9bba4 100644 --- a/dev/bots/test/test_test.dart +++ b/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 makeTests(int count) => List.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 tests = makeTests(testCount); + final List> subshards = List>.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 testedTests = subshards.flattened.toList(); + final Set deduped = Set.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); + }); } diff --git a/dev/bots/utils.dart b/dev/bots/utils.dart index 735b5ef3e7..90050841b9 100644 --- a/dev/bots/utils.dart +++ b/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 runShardRunnerIndexOfTotalSubshard(List 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 selectIndexOfTotalSubshard(List tests, {String subshardKey = kSubs return []; } - 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 buckets = List.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 _runFromList(Map items, String key, String name, int positionInTaskName) async { String? item = Platform.environment[key]; if (item == null && Platform.environment.containsKey(CIRRUS_TASK_NAME)) {