pkg/dartdev/test/commands/test_test broke (Pass -> RuntimeError, expected Pass) on 2.13.0-211.13.beta

[athomas]

The test started failing with the latest round of cherry-picks without a clear root cause. We should investigate if this is a release blocker for 2.13 stable or not.

The diff can be seen on the merge commit: 5cae07b

The error happened on Linux, Mac and windows:
https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8848887849855904544/+/u/test_results/new_test_failures__logs_

/=================================================================================\
 | pkg/dartdev/test/commands/test_test broke (Pass -> RuntimeError, expected Pass) |
 \=================================================================================/
 --- Command "vm" (took 04:57.000448s):
 DART_CONFIGURATION=ReleaseX64 CHROME_PATH="/b/s/w/ir/cache/builder/sdk/third_party/browsers/chrome/Google Chrome.app/Contents/MacOS/Google Chrome" xcodebuild/ReleaseX64/dart-sdk/bin/dart --enable_asserts --ignore-unrecognized-flags --packages=/b/s/w/ir/cache/builder/sdk/.packages /b/s/w/ir/cache/builder/sdk/pkg/dartdev/test/commands/test_test.dart
 exit code:
 255
 stdout:
 00:00 �[32m+0�[0m: test --help�[0m
 00:26 �[32m+1�[0m: test dart help test�[0m
 00:26 �[32m+2�[0m: test no pubspec.yaml�[0m
 00:27 �[32m+3�[0m: test runs test�[0m
 01:16 �[32m+4�[0m: test no package:test dependency�[0m
 02:00 �[32m+5�[0m: test has package:test dependency�[0m
 02:36 �[32m+6�[0m: test --enable-experiment triple-shift�[0m
 03:23 �[32m+6�[0m�[31m -1�[0m: test --enable-experiment triple-shift �[1m�[31m[E]�[0m�[0m
   Expected: <0>
     Actual: <1>
   stdout: 00:00 +0 -1: loading test/experiment_test.dart [E]
     Failed to load "test/experiment_test.dart":
     lib/main.dart:2:11: Error: A method declaration needs an explicit list of parameters.
     Try adding a parameter list to the method declaration.
       operator>>>(int k) => 42;
               ^^
     lib/main.dart:2:13: Error: Expected '{' before this.
       operator>>>(int k) => 42;
                 ^
     lib/main.dart:2:11: Error: Operator '>>' should have exactly one parameter.
       operator>>>(int k) => 42;
               ^^
     lib/main.dart:2:13: Error: Operator declarations must be preceded by the keyword 'operator'.
     Try adding the keyword 'operator'.
       operator>>>(int k) => 42;
                 ^
     lib/main.dart:5:14: Error: Expected an identifier, but got '>'.
     Try inserting an identifier before '>'.
       if ((A() >>> 1) == 42) print('feature enabled');
                  ^
   00:00 +0 -1: Some tests failed.
    stderr: 
   
   package:test_api/src/frontend/expect.dart 153:31               fail
   package:test_api/src/frontend/expect.dart 148:3                _expect
   package:test_api/src/frontend/expect.dart 57:3                 expect
   pkg/dartdev/test/commands/test_test.dart 166:7                 defineTest.<fn>.expectSuccess
   pkg/dartdev/test/commands/test_test.dart 198:24                defineTest.<fn>.<fn>
   package:test_api/src/backend/declarer.dart 200:19              Declarer.test.<fn>.<fn>
   ===== asynchronous gap ===========================
   dart:async/zone.dart 1294:19                                   _CustomZone.registerBinaryCallback
   dart:async-patch/async_patch.dart 61:8                         _asyncErrorWrapperHelper
   package:test_api/src/backend/invoker.dart                      Invoker.waitForOutstandingCallbacks.<fn>
   dart:async/zone.dart 1354:13                                   _rootRun
   dart:async/zone.dart 1258:19                                   _CustomZone.run
   dart:async/zone.dart 1789:10                                   _runZoned
   dart:async/zone.dart 1711:10                                   runZoned
   package:test_api/src/backend/invoker.dart 228:5                Invoker.waitForOutstandingCallbacks
   package:test_api/src/backend/invoker.dart 383:17               Invoker._onRun.<fn>.<fn>.<fn>
   ===== asynchronous gap ===========================
   dart:async/zone.dart 1286:19                                   _CustomZone.registerUnaryCallback
   dart:async-patch/async_patch.dart 45:22                        _asyncThenWrapperHelper
   package:test_api/src/backend/invoker.dart                      Invoker._onRun.<fn>.<fn>.<fn>
   dart:async/zone.dart 1354:13                                   _rootRun
   dart:async/zone.dart 1258:19                                   _CustomZone.run
   dart:async/zone.dart 1789:10                                   _runZoned
   dart:async/zone.dart 1711:10                                   runZoned
   package:test_api/src/backend/invoker.dart 370:9                Invoker._onRun.<fn>.<fn>
   package:test_api/src/backend/invoker.dart 415:15               Invoker._guardIfGuarded
   package:test_api/src/backend/invoker.dart 369:7                Invoker._onRun.<fn>
   package:stack_trace/src/chain.dart 94:24                       Chain.capture.<fn>
   dart:async/zone.dart 1354:13                                   _rootRun
   dart:async/zone.dart 1258:19                                   _CustomZone.run
   dart:async/zone.dart 1789:10                                   _runZoned
   dart:async/zone.dart 1711:10                                   runZoned
   package:stack_trace/src/chain.dart 92:12                       Chain.capture
   package:test_api/src/backend/invoker.dart 368:11               Invoker._onRun
   package:test_api/src/backend/live_test_controller.dart 153:11  LiveTestController.run
   dart:async/future.dart 198:37                                  new Future.microtask.<fn>
   dart:async/zone.dart 1346:47                                   _rootRun
   dart:async/zone.dart 1258:19                                   _CustomZone.run
   dart:async/zone.dart 1162:7                                    _CustomZone.runGuarded
   dart:async/zone.dart 1202:23                                   _CustomZone.bindCallbackGuarded.<fn>
   dart:async/zone.dart 1354:13                                   _rootRun
   dart:async/zone.dart 1258:19                                   _CustomZone.run
   dart:async/zone.dart 1162:7                                    _CustomZone.runGuarded
   dart:async/zone.dart 1202:23                                   _CustomZone.bindCallbackGuarded.<fn>
   dart:async/schedule_microtask.dart 40:21                       _microtaskLoop
   dart:async/schedule_microtask.dart 49:5                        _startMicrotaskLoop
   dart:isolate-patch/isolate_patch.dart 120:13                   _runPendingImmediateCallback
   dart:isolate-patch/timer_impl.dart 402:11                      _Timer._runTimers
   dart:isolate-patch/timer_impl.dart 426:5                       _Timer._handleMessage
   dart:isolate-patch/isolate_patch.dart 184:12                   _RawReceivePortImpl._handleMessage
   
 03:23 �[32m+6�[0m�[31m -1�[0m: test --enable-experiment nonfunction-type-aliases�[0m
 04:06 �[32m+7�[0m�[31m -1�[0m: test --enable-experiment non-nullable�[0m
 04:54 �[32m+8�[0m�[31m -1�[0m: �[31mSome tests failed.�[0m
 stderr:
 Unhandled exception:
 Dummy exception to set exit code.
 --- Re-run this test:
 python tools/test.py -n unittest-asserts-release-mac pkg/dartdev/test/commands/test_test

[vsmenon]

This looks like the CFE doesn't know that triple shift was enabled.

Did we have any plumbing in the test runner for this? Triple shift was just enabled. @leafpetersen @johnniwinther

[bkonyi]

I've added some prints to see which arguments are being passed where:

00:21 +0 -1: test --enable-experiment triple-shift [E]

  Expected: <0>
    Actual: <1>
  stdout: 00:00 +0 -1: loading test/experiment_test.dart [E]
    Failed to load "test/experiment_test.dart":
    lib/main.dart:2:11: Error: A method declaration needs an explicit list of parameters.
    Try adding a parameter list to the method declaration.
      operator>>>(int k) => 42;
              ^^
    lib/main.dart:2:13: Error: Expected '{' before this.
      operator>>>(int k) => 42;
                ^
    lib/main.dart:2:11: Error: Operator '>>' should have exactly one parameter.
      operator>>>(int k) => 42;
              ^^
    lib/main.dart:2:13: Error: Operator declarations must be preceded by the keyword 'operator'.
    Try adding the keyword 'operator'.
      operator>>>(int k) => 42;
                ^
    lib/main.dart:5:14: Error: Expected an identifier, but got '>'.
    Try inserting an identifier before '>'.
      if ((A() >>> 1) == 42) print('feature enabled');
                 ^
  00:00 +0 -1: Some tests failed.
   stderr: 
  // dart --enable-experiment=triple-shift test --no-color --reporter expanded
  VMArg[0] = --new_gen_semi_max_size=32
  VMArg[1] = --new_gen_growth_factor=4
  VMArg[2] = --enable-experiment=triple-shift
  Script: .dart_tool/pub/bin/test/test.dart-2.13.0-211.13.beta.snapshot
  Arg[0] = --no-color
  Arg[1] = --reporter
  Arg[2] = expanded
  
  // Pub test compilation
  VMArg[0] = --new_gen_semi_max_size=32
  VMArg[1] = --new_gen_growth_factor=4
  Script: /usr/local/google/home/bkonyi/sdk/out/ReleaseX64/dart-sdk/bin/snapshots/frontend_server.dart.snapshot
  Arg[0] = --sdk-root
  Arg[1] = ../../usr/local/google/home/bkonyi/sdk/out/ReleaseX64/dart-sdk
  Arg[2] = --platform=lib/_internal/vm_platform_strong.dill
  Arg[3] = --target=vm
  Arg[4] = --filesystem-scheme
  Arg[5] = org-dartlang-root
  Arg[6] = --output-dill
  Arg[7] = /tmp/dart_test.BUOSHB/output.dill
  Arg[8] = --packages=/tmp/aRZYGTQ/.dart_tool/package_config.json
  Arg[9] = --incremental
  Arg[10] = --no-print-incremental-dependencies

We're not passing --enable-experiment=triple-shift to the frontend when compiling the test. Assigning to dart test owners to investigate.

[a-siva]

Looks like this is just an option issue and not a regression. Since this new feature is not part of the beta release we can lower the priority to P3.

[natebosch]

The flag should be passed to the VM, not to the test runner. We forward experiments from Platform.executableArguments.

[natebosch]

Or maybe this didn't get wired up with the frontend_server? @jakemac53

[jakemac53]

Hmm ya I don't think this ever got wired up. You can revert to the isolate strategy by using the --use-data-isolate-strategy flag in test runner invocations.

The way this was supported previously is really not something I think we should double down on... it forces you to enable the experiment for the test runner in order to enable them in your tests, as well as not allowing you to configure the setting per test.

I would honestly prefer to rip out that support entirely instead of re-implementing that behavior?

[natebosch]

Should we consider adding an explicit test runner argument for experiments to enable within tests?

[jakemac53]

Should we consider adding an explicit test runner argument for experiments to enable within tests?

I think ideally it would be test metadata, something you can enable per file. That also means creating yet another frontend server compiler for each combination of experiments but 🤷.

You could enable it for all tests in dart_test.yml.