Reapply "[test_runner] Improve timeout deflaking" The fix is in https://dart-review.googlesource.com/c/sdk/+/441460/1..2 The issue was that the TestConfiguration.selectors field was set incorrectly, because _expandSelectors was still reading configuration['test-list-contents'] without accounting for the format changes. It just needs the test name list, so the fix is to pass in the already parsed testList variable. Example build: https://ci.chromium.org/ui/p/dart/builders/try/vm-aot-linux-release-x64-try/7213/overview compare_results output: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8708774547991696017/+/u/deflaking/list_tests_to_deflake__vm_tests_/raw_io.output_text Deflaking run flags: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8708774547991696017/+/u/deflaking/vm_tests/l_execution_details Deflaking run output: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8708774547991696017/+/u/deflaking/vm_tests/stdout This reverts commit cc5f351c3499bf06b0468f481e2a76d3a236f28e. TEST=CI Bug: https://github.com/dart-lang/sdk/issues/55044 Change-Id: Ie9ff1ba71f44ade7b4cd1c9b2f75b9efc1dee24a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441460 Commit-Queue: Liam Appelbe <liama@google.com> Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/pkg/test_runner/bin/compare_results.dart b/pkg/test_runner/bin/compare_results.dart index 10ecbf2..77f1c6c 100755 --- a/pkg/test_runner/bin/compare_results.dart +++ b/pkg/test_runner/bin/compare_results.dart
@@ -8,10 +8,12 @@ // readable mode that explains the results and how they changed. import 'dart:collection'; +import 'dart:convert'; import 'dart:io'; import 'package:args/args.dart'; import 'package:test_runner/bot_results.dart'; +import 'package:test_runner/src/deflake_info.dart'; class Event { final Result? before; @@ -50,6 +52,18 @@ throw Exception("Unreachable"); } } + + String deflakeInfo(String name) { + final isTimeout = after.outcome == 'Timeout'; + final lastTimeMs = before?.timeMs; + return jsonEncode(DeflakeInfo( + name: name, + repeat: isTimeout ? 2 : 5, + timeout: isTimeout && lastTimeMs != null + ? ((2 * lastTimeMs) / 1000).ceil() + : -1, + ).toJson()); + } } class Options { @@ -70,6 +84,7 @@ Iterable<String> get statusFilter => ["passing", "flaky", "failing"] .where((option) => _options[option] as bool); bool get unchanged => _options["unchanged"] as bool; + bool get nameOnly => _options["name-only"] as bool; bool get verbose => _options["verbose"] as bool; List<String> get rest => _options.rest; } @@ -148,8 +163,10 @@ "${before?.matches} ${after.matches} " "${before?.flaked} ${after.flaked}"; } - } else { + } else if (options.nameOnly) { output = name; + } else { + output = event.deflakeInfo(name); } final log = logs[event.after.key]; final bar = '=' * (output.length + 2); @@ -191,6 +208,8 @@ abbr: 'u', negatable: false, help: "Show only tests with unchanged results."); + parser.addFlag("name-only", + help: "Only show the test names.", negatable: false); parser.addFlag("verbose", abbr: "v", help: "Show the old and new result for each test",
diff --git a/pkg/test_runner/lib/bot_results.dart b/pkg/test_runner/lib/bot_results.dart index b313e0a..ac7cbb4 100644 --- a/pkg/test_runner/lib/bot_results.dart +++ b/pkg/test_runner/lib/bot_results.dart
@@ -30,6 +30,7 @@ final bool flaked; // From optional flakiness_data argument to constructor. final bool? isFlaky; // From results.json after it is extended. final String? previousOutcome; + final int? timeMs; Result( this.configuration, @@ -40,7 +41,8 @@ this.changed, this.commitHash, this.isFlaky, - this.previousOutcome, { + this.previousOutcome, + this.timeMs, { this.flaked = false, }); @@ -56,6 +58,7 @@ commitHash = map["commit_hash"] as String?, isFlaky = map["flaky"] as bool?, previousOutcome = map["previous_result"] as String?, + timeMs = map["timeMs"] as int?, flaked = flakinessData != null && (flakinessData["active"] ?? true) == true && (flakinessData["outcomes"] as List).contains(map["result"]);
diff --git a/pkg/test_runner/lib/src/configuration.dart b/pkg/test_runner/lib/src/configuration.dart index b9311e0..5183c38 100644 --- a/pkg/test_runner/lib/src/configuration.dart +++ b/pkg/test_runner/lib/src/configuration.dart
@@ -11,6 +11,7 @@ import 'package:smith/smith.dart'; import 'compiler_configuration.dart'; +import 'deflake_info.dart'; import 'feature.dart'; import 'path.dart'; import 'repository.dart'; @@ -31,7 +32,8 @@ this.selectors = const {}, this.build = false, this.testList = const [], - this.repeat = 1, + this.deflakeInfoMap = const {}, + int repeat = 1, this.batch = false, this.copyCoreDumps = false, this.rr = false, @@ -74,7 +76,8 @@ : packages = packages ?? Repository.uri .resolve('.dart_tool/package_config.json') - .toFilePath(); + .toFilePath(), + _repeat = repeat; final Map<String, RegExp?> selectors; final Progress progress; @@ -136,11 +139,12 @@ final String? dartPrecompiledPath; final String? genSnapshotPath; final List<String>? testList; + final Map<String, DeflakeInfo> deflakeInfoMap; final int taskCount; final int shardCount; final int shard; - final int repeat; + final int _repeat; final int testServerPort; final int testServerCrossOriginPort; @@ -221,15 +225,15 @@ /// build/ReleaseX64 String get buildDirectory => system.outputDirectory + configurationDirectory; - int? _timeout; + int? _defaultTimeout; // TODO(whesse): Put non-default timeouts explicitly in configs, not this. /// Calculates a default timeout based on the compiler and runtime used, /// and the mode, architecture, etc. - int get timeout { - if (_timeout == null) { + int get defaultTimeout { + if (_defaultTimeout == null) { if (configuration.timeout > 0) { - _timeout = configuration.timeout; + _defaultTimeout = configuration.timeout; } else { var isReload = hotReload || hotReloadRollback; @@ -241,13 +245,22 @@ arch: architecture, system: system); - _timeout = 30 * compilerMultiplier * runtimeMultiplier; + _defaultTimeout = 30 * compilerMultiplier * runtimeMultiplier; } } - return _timeout!; + return _defaultTimeout!; } + /// Returns the timeout for the given test name. + int timeout(String name) { + final t = deflakeInfoMap[name]?.timeout ?? -1; + return t >= 0 ? t : defaultTimeout; + } + + /// Returns the repeat count for the given test name. + int repeat(String name) => deflakeInfoMap[name]?.repeat ?? _repeat; + List<String> get standardOptions { if (compiler != Compiler.dart2js) { return const ["--ignore-unrecognized-flags"];
diff --git a/pkg/test_runner/lib/src/deflake_info.dart b/pkg/test_runner/lib/src/deflake_info.dart new file mode 100644 index 0000000..807e30b --- /dev/null +++ b/pkg/test_runner/lib/src/deflake_info.dart
@@ -0,0 +1,25 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +/// During deflaking, each test can be run with a custom timeout and repetition +/// count. +class DeflakeInfo { + String name; + int repeat; + int timeout; + + DeflakeInfo( + {required this.name, required this.repeat, required this.timeout}); + + Map<dynamic, dynamic> toJson() => { + 'name': name, + 'repeat': repeat, + 'timeout': timeout, + }; + + static DeflakeInfo fromJson(Map<dynamic, dynamic> json) => DeflakeInfo( + name: json['name'] as String, + repeat: json['repeat'] as int? ?? 5, + timeout: json['timeout'] as int? ?? -1); +}
diff --git a/pkg/test_runner/lib/src/options.dart b/pkg/test_runner/lib/src/options.dart index 300a4ae..2d578ca 100644 --- a/pkg/test_runner/lib/src/options.dart +++ b/pkg/test_runner/lib/src/options.dart
@@ -11,6 +11,7 @@ import 'package:path/path.dart' as path; import 'configuration.dart'; +import 'deflake_info.dart'; import 'path.dart'; import 'repository.dart'; import 'test_configurations.dart'; @@ -626,12 +627,16 @@ void addConfiguration(Configuration innerConfiguration, [String? namedConfiguration]) { + final (testList, deflakeInfoMap) = + parseTestList(data["test-list-contents"] as List<String>?); + var configuration = TestConfiguration( configuration: innerConfiguration, progress: progress, - selectors: _expandSelectors(data), + selectors: _expandSelectors(data, testList), build: data["build"] as bool, - testList: data["test-list-contents"] as List<String>?, + testList: testList, + deflakeInfoMap: deflakeInfoMap ?? const {}, repeat: int.parse(data["repeat"] as String), batch: !(data["no-batch"] as bool), copyCoreDumps: data["copy-coredumps"] as bool, @@ -812,18 +817,15 @@ /// /// If no selectors are explicitly given, uses the default suite patterns. Map<String, RegExp> _expandSelectors( - Map<String, dynamic> configuration) { + Map<String, dynamic> configuration, List<String>? testList) { var selectors = configuration['selectors'] as List<String>? ?? []; if (selectors.isEmpty || configuration['default-suites'] as bool) { if (configuration['suite-dir'] != null) { var suitePath = Path(configuration['suite-dir'] as String); selectors.add(suitePath.filename); - } else if (configuration['test-list-contents'] != null) { - selectors = (configuration['test-list-contents'] as List<String>) - .map((t) => t.split('/').first) - .toSet() - .toList(); + } else if (testList != null) { + selectors = testList.map((t) => t.split('/').first).toSet().toList(); } else { selectors.addAll(_defaultTestSelectors); } @@ -1009,3 +1011,16 @@ return environment; })(); + +(List<String>?, Map<String, DeflakeInfo>?) parseTestList(List<String>? raw) { + final isJson = raw != null && raw.isNotEmpty && raw[0].startsWith('{'); + if (!isJson) return (raw, null); + final deflakes = <DeflakeInfo>[ + for (var line in raw) + DeflakeInfo.fromJson(jsonDecode(line) as Map<dynamic, dynamic>), + ]; + return ( + [for (var i in deflakes) i.name], + {for (var i in deflakes) i.name: i}, + ); +}
diff --git a/pkg/test_runner/lib/src/process_queue.dart b/pkg/test_runner/lib/src/process_queue.dart index a426b75..775de22 100644 --- a/pkg/test_runner/lib/src/process_queue.dart +++ b/pkg/test_runner/lib/src/process_queue.dart
@@ -253,7 +253,7 @@ /// test completing successfully, just on it completing. void _add(TestCase testCase) { Node<Command>? lastNode; - for (var i = 0; i < testCase.configuration.repeat; ++i) { + for (var i = 0; i < testCase.repeat; ++i) { if (i > 0) { testCase = testCase.indexedCopy(i); }
diff --git a/pkg/test_runner/lib/src/test_case.dart b/pkg/test_runner/lib/src/test_case.dart index 1633e1a..5164ae9 100644 --- a/pkg/test_runner/lib/src/test_case.dart +++ b/pkg/test_runner/lib/src/test_case.dart
@@ -133,7 +133,7 @@ } int get timeout { - var result = configuration.timeout; + var result = configuration.timeout(displayName); if (expectedOutcomes.contains(Expectation.slow)) { result *= _slowTimeoutMultiplier; } else if (expectedOutcomes.contains(Expectation.extraSlow)) { @@ -142,6 +142,8 @@ return result; } + int get repeat => configuration.repeat(displayName); + String get configurationString { var compiler = configuration.compiler.name; var runtime = configuration.runtime.name;
diff --git a/pkg/test_runner/lib/test_runner.dart b/pkg/test_runner/lib/test_runner.dart index b1f25c3..860028c 100644 --- a/pkg/test_runner/lib/test_runner.dart +++ b/pkg/test_runner/lib/test_runner.dart
@@ -577,6 +577,7 @@ // Find the list of tests to deflake. var deflakeListOutput = await runProcess(Platform.resolvedExecutable, [ "tools/bots/compare_results.dart", + "--name-only", "--changed", "--failing", "--passing",
diff --git a/pkg/test_runner/test/compare_results/compare_results_test.dart b/pkg/test_runner/test/compare_results/compare_results_test.dart index c655ddf..5318cf9 100644 --- a/pkg/test_runner/test/compare_results/compare_results_test.dart +++ b/pkg/test_runner/test/compare_results/compare_results_test.dart
@@ -197,7 +197,8 @@ String commitHash = 'abcdabcd', bool flaked = false, bool isFlaky = false, - String previousOutcome = 'Pass'}) { + String previousOutcome = 'Pass', + int timeMs = -1}) { return Result( configuration, name, @@ -208,6 +209,7 @@ commitHash, isFlaky, previousOutcome, + timeMs, flaked: flaked, ); }