Revert "[test_runner] Improve timeout deflaking"

This reverts commit f95a84d.

Reason for revert: Testing if CL caused increased meta-flakiness of non-timeout failures.

Bug: #55044
Original change's description:
> [test_runner] Improve timeout deflaking
>
> The usual deflaking logic is to simply rerun the test 5 times, but if the failure was a timeout this can be very expensive. Instead, only rerun timeout failures twice, and use the last successful run time to set a tighter timeout.
>
> Repeat counts and timeouts are now per-test. The name, repeat count, and timeout make a `DeflakeInfo`. This object is constructed in compare_results.dart, and passed to test.py's `--tests` flag. The recipe was already passing the output of compare_results directly to that flag, so no change is needed to the recipies.
>
> Backwards compatibility:
>
> If you want the old behavior of compare_results.dart, use the `--name-only` flag.
>
> test.py only uses this new logic if it detects that the `--tests` flag is JSON (if it starts with a `{`).
>
> Change-Id: I4113b68c54bfb7fd9a5e8fc9dab7a265807f3e77
> Bug: #55044
> Fixes: #55044
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435760
> Commit-Queue: Liam Appelbe <[email protected]>
> Reviewed-by: Alexander Thomas <[email protected]>

Bug: #55044
Change-Id: I68268dcaffbe857ccec20e211c34325c03643b37
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439180
Commit-Queue: Liam Appelbe <[email protected]>
Reviewed-by: Siva Annamalai <[email protected]>

Author: liamappelbe

pkg/test_runner/bin/compare_results.dart

@@ -8,12 +8,10 @@
 // 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;
@@ -52,18 +50,6 @@ class Event {
       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 {
@@ -84,7 +70,6 @@ class Options {
   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;
 }
@@ -163,10 +148,8 @@ bool search(
             "${before?.matches} ${after.matches} "
             "${before?.flaked} ${after.flaked}";
       }
-    } else if (options.nameOnly) {
-      output = name;
     } else {
-      output = event.deflakeInfo(name);
+      output = name;
     }
     final log = logs[event.after.key];
     final bar = '=' * (output.length + 2);
@@ -208,8 +191,6 @@ void main(List<String> args) async {
       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",

pkg/test_runner/lib/bot_results.dart

@@ -30,7 +30,6 @@ class Result {
   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,
@@ -41,8 +40,7 @@ class Result {
     this.changed,
     this.commitHash,
     this.isFlaky,
-    this.previousOutcome,
-    this.timeMs, {
+    this.previousOutcome, {
     this.flaked = false,
   });
 
@@ -58,7 +56,6 @@ class Result {
         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"]);

pkg/test_runner/lib/src/configuration.dart

@@ -11,7 +11,6 @@ import 'package:smith/configuration.dart';
 import 'package:smith/smith.dart';
 
 import 'compiler_configuration.dart';
-import 'deflake_info.dart';
 import 'feature.dart';
 import 'path.dart';
 import 'repository.dart';
@@ -32,8 +31,7 @@ class TestConfiguration {
       this.selectors = const {},
       this.build = false,
       this.testList = const [],
-      this.deflakeInfoMap = const {},
-      int repeat = 1,
+      this.repeat = 1,
       this.batch = false,
       this.copyCoreDumps = false,
       this.rr = false,
@@ -76,8 +74,7 @@ class TestConfiguration {
       : packages = packages ??
             Repository.uri
                 .resolve('.dart_tool/package_config.json')
-                .toFilePath(),
-        _repeat = repeat;
+                .toFilePath();
 
   final Map<String, RegExp?> selectors;
   final Progress progress;
@@ -139,12 +136,11 @@ class TestConfiguration {
   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;
@@ -225,15 +221,15 @@ class TestConfiguration {
   ///     build/ReleaseX64
   String get buildDirectory => system.outputDirectory + configurationDirectory;
 
-  int? _defaultTimeout;
+  int? _timeout;
 
   // 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 defaultTimeout {
-    if (_defaultTimeout == null) {
+  int get timeout {
+    if (_timeout == null) {
       if (configuration.timeout > 0) {
-        _defaultTimeout = configuration.timeout;
+        _timeout = configuration.timeout;
       } else {
         var isReload = hotReload || hotReloadRollback;
 
@@ -245,22 +241,13 @@ class TestConfiguration {
             arch: architecture,
             system: system);
 
-        _defaultTimeout = 30 * compilerMultiplier * runtimeMultiplier;
+        _timeout = 30 * compilerMultiplier * runtimeMultiplier;
       }
     }
 
-    return _defaultTimeout!;
+    return _timeout!;
   }
 
-  /// 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"];

pkg/test_runner/lib/src/deflake_info.dart

@@ -11,7 +11,6 @@ import 'package:args/args.dart';
 import 'package:path/path.dart' as path;
 
 import 'configuration.dart';
-import 'deflake_info.dart';
 import 'path.dart';
 import 'repository.dart';
 import 'test_configurations.dart';
@@ -627,17 +626,12 @@ has been specified on the command line.''')
 
     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),
           build: data["build"] as bool,
-          testList: testList,
-          deflakeInfoMap: deflakeInfoMap ?? const {},
+          testList: data["test-list-contents"] as List<String>?,
           repeat: int.parse(data["repeat"] as String),
           batch: !(data["no-batch"] as bool),
           copyCoreDumps: data["copy-coredumps"] as bool,
@@ -1015,16 +1009,3 @@ final Map<String, String> sanitizerEnvironmentVariables = (() {
 
   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 },
-  );
-}

pkg/test_runner/lib/src/options.dart

@@ -253,7 +253,7 @@ class TestCaseEnqueuer {
   /// test completing successfully, just on it completing.
   void _add(TestCase testCase) {
     Node<Command>? lastNode;
-    for (var i = 0; i < testCase.repeat; ++i) {
+    for (var i = 0; i < testCase.configuration.repeat; ++i) {
       if (i > 0) {
         testCase = testCase.indexedCopy(i);
       }

pkg/test_runner/lib/src/process_queue.dart

@@ -133,7 +133,7 @@ class TestCase {
   }
 
   int get timeout {
-    var result = configuration.timeout(displayName);
+    var result = configuration.timeout;
     if (expectedOutcomes.contains(Expectation.slow)) {
       result *= _slowTimeoutMultiplier;
     } else if (expectedOutcomes.contains(Expectation.extraSlow)) {
@@ -142,8 +142,6 @@ class TestCase {
     return result;
   }
 
-  int get repeat => configuration.repeat(displayName);
-
   String get configurationString {
     var compiler = configuration.compiler.name;
     var runtime = configuration.runtime.name;

pkg/test_runner/lib/src/test_case.dart

@@ -577,7 +577,6 @@ Future<void> deflake(Directory outDirectory, List<String> configurations,
   // Find the list of tests to deflake.
   var deflakeListOutput = await runProcess(Platform.resolvedExecutable, [
     "tools/bots/compare_results.dart",
-    "--name-only",
     "--changed",
     "--failing",
     "--passing",

pkg/test_runner/lib/test_runner.dart

@@ -197,8 +197,7 @@ Result _result(
     String commitHash = 'abcdabcd',
     bool flaked = false,
     bool isFlaky = false,
-    String previousOutcome = 'Pass',
-    int timeMs = -1}) {
+    String previousOutcome = 'Pass'}) {
   return Result(
     configuration,
     name,
@@ -209,7 +208,6 @@ Result _result(
     commitHash,
     isFlaky,
     previousOutcome,
-    timeMs,
     flaked: flaked,
   );
 }