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,
);
}
