[test_runner] Remove nnbd mode options.

In particular, this means the VM tests stop getting invoked with the now-invalid --sound-null-safety flag.

Change-Id: Ia8f5a59e0bf1fd7094508fdc4bb146ebf3034148
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/431360
Reviewed-by: Bob Nystrom <[email protected]>
Reviewed-by: Kallen Tu <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

pkg/test_runner/lib/src/compiler_configuration.dart

@@ -34,19 +34,6 @@ List<String> _experimentsArgument(
   return ['--enable-experiment=${experiments.join(',')}'];
 }
 
-List<String> _nnbdModeArgument(TestConfiguration configuration) {
-  switch (configuration.nnbdMode) {
-    case NnbdMode.legacy:
-      return [];
-    case NnbdMode.strong:
-      return ['--sound-null-safety'];
-    case NnbdMode.weak:
-      return ['--no-sound-null-safety'];
-  }
-
-  throw 'unreachable';
-}
-
 /// Grouping of a command with its expected result.
 class CommandArtifact {
   final List<Command> commands;
@@ -193,7 +180,6 @@ class NoneCompilerConfiguration extends CompilerConfiguration {
       else if (_configuration.hotReloadRollback)
         '--hot-reload-rollback-test-mode',
       ...vmOptions,
-      ..._nnbdModeArgument(_configuration),
       ...testFile.sharedOptions,
       ..._configuration.sharedOptions,
       ..._experimentsArgument(_configuration, testFile),
@@ -249,7 +235,6 @@ class VMKernelCompilerConfiguration extends CompilerConfiguration
       ..._configuration.sharedOptions,
       ..._experimentsArgument(_configuration, testFile),
       ...vmOptions,
-      ..._nnbdModeArgument(_configuration),
       ...args
     ];
   }
@@ -275,7 +260,6 @@ class VMKernelCompilerConfiguration extends CompilerConfiguration
       else if (_configuration.hotReloadRollback)
         '--hot-reload-rollback-test-mode',
       ...vmOptions,
-      ..._nnbdModeArgument(_configuration),
       ...testFile.sharedOptions,
       ..._configuration.sharedOptions,
       ..._experimentsArgument(_configuration, testFile),
@@ -985,7 +969,6 @@ class PrecompilerCompilerConfiguration extends CompilerConfiguration
         '--no-sim-use-hardfp',
       ],
       if (_configuration.isMinified) '--obfuscate',
-      ..._nnbdModeArgument(_configuration),
       if (arguments.contains('--print-flow-graph-optimized'))
         '--redirect-isolate-log-to=$tempDir/out.il',
       if (arguments.contains('--print-flow-graph-optimized') &&
@@ -1192,7 +1175,6 @@ class PrecompilerCompilerConfiguration extends CompilerConfiguration
     return [
       if (_enableAsserts) '--enable_asserts',
       ...vmOptions,
-      ..._nnbdModeArgument(_configuration),
       ...testFile.sharedOptions,
       ..._configuration.sharedOptions,
       ..._experimentsArgument(_configuration, testFile),
@@ -1254,7 +1236,6 @@ class AppJitCompilerConfiguration extends CompilerConfiguration {
     return [
       if (_enableAsserts) '--enable_asserts',
       ...vmOptions,
-      ..._nnbdModeArgument(_configuration),
       ...testFile.sharedOptions,
       ..._configuration.sharedOptions,
       ..._experimentsArgument(_configuration, testFile),
@@ -1273,7 +1254,6 @@ class AppJitCompilerConfiguration extends CompilerConfiguration {
     return [
       if (_enableAsserts) '--enable_asserts',
       ...vmOptions,
-      ..._nnbdModeArgument(_configuration),
       ...testFile.sharedOptions,
       ..._configuration.sharedOptions,
       ..._experimentsArgument(_configuration, testFile),
@@ -1427,7 +1407,6 @@ abstract mixin class VMKernelCompilerMixin {
           arguments.contains('--enable-asserts') ||
           arguments.contains('--enable_asserts'))
         '--enable-asserts',
-      ..._nnbdModeArgument(_configuration),
       ..._configuration.genKernelOptions,
     ];
 

pkg/test_runner/lib/src/configuration.dart

@@ -109,7 +109,6 @@ class TestConfiguration {
   Mode get mode => configuration.mode;
   Runtime get runtime => configuration.runtime;
   System get system => configuration.system;
-  NnbdMode get nnbdMode => configuration.nnbdMode;
   Sanitizer get sanitizer => configuration.sanitizer;
 
   // Boolean getters
@@ -408,23 +407,15 @@ class TestConfiguration {
 
   /// The set of [Feature]s supported by this configuration.
   Set<Feature> get supportedFeatures {
-    // The analyzer should parse all tests that don't require legacy support.
+    // The analyzer should handle all tests.
     if (compiler == Compiler.dart2analyzer) {
-      return {...Feature.all.where((f) => !Feature.legacy.contains(f))};
+      return {...Feature.all};
     }
 
     var isDart2jsProduction = dart2jsOptions.contains('-O3');
     var isOptimizedDart2Wasm = dart2wasmOptions.contains('-O1');
     var isJsCompiler = compiler == Compiler.dart2js || compiler == Compiler.ddc;
     return {
-      // The supported NNBD features depending on the `nnbdMode`.
-      if (NnbdMode.legacy == configuration.nnbdMode)
-        Feature.nnbdLegacy
-      else
-        Feature.nnbd,
-      if (NnbdMode.weak == configuration.nnbdMode) Feature.nnbdWeak,
-      if (NnbdMode.strong == configuration.nnbdMode) Feature.nnbdStrong,
-
       // The configurations with the following builder tags and configurations
       // with the `minified` flag set to `true` will obfuscate `Type.toString`
       // strings.

pkg/test_runner/lib/src/environment.dart

@@ -26,7 +26,6 @@ final _variables = {
   "jscl": _Variable.bool((c) => c.runtime.isJSCommandLine),
   "minified": _Variable.bool((c) => c.isMinified),
   "mode": _Variable((c) => c.mode.name, Mode.names),
-  "nnbd": _Variable((TestConfiguration c) => c.nnbdMode.name, NnbdMode.names),
   "qemu": _Variable.bool((c) => c.useQemu),
   "runtime": _Variable(_runtimeName, _runtimeNames),
   "sanitizer": _Variable((c) => c.sanitizer.name, Sanitizer.names),

pkg/test_runner/lib/src/feature.dart

@@ -36,21 +36,6 @@ class Feature {
   /// Supports native number semantics.
   static const nativeNumbers = Feature._("native-numbers");
 
-  /// Opted out of NNBD and still using the legacy semantics.
-  static const nnbdLegacy = Feature._("nnbd-legacy");
-
-  /// Opted in to NNBD features.
-  ///
-  /// Note that this does not imply either strong or weak checking. A test that
-  /// only requires "nnbd" should run in both weak and strong checking modes.
-  static const nnbd = Feature._("nnbd");
-
-  /// Weak checking of NNBD features.
-  static const nnbdWeak = Feature._("nnbd-weak");
-
-  /// Full strong checking of NNBD features.
-  static const nnbdStrong = Feature._("nnbd-strong");
-
   /// Expects [Type.toString] to show the original type name and original
   /// names in function type named parameters.
   static const readableTypeStrings = Feature._("readable-type-strings");
@@ -61,16 +46,9 @@ class Feature {
     checkedParameters,
     jsNumbers,
     nativeNumbers,
-    nnbdLegacy,
-    nnbd,
-    nnbdWeak,
-    nnbdStrong,
     readableTypeStrings,
   ];
 
-  /// All modes that should be tested on a platform that support legacy code.
-  static const legacy = [nnbdLegacy, nnbdWeak];
-
   final String name;
 
   const Feature._(this.name);

pkg/test_runner/lib/src/options.dart

@@ -250,10 +250,6 @@ test options, specifying how tests should be run.''')
         hide: true, help: 'Path to safari browser executable.')
     ..addFlag('use-sdk',
         aliases: ['use_sdk'], help: 'Use compiler or runtime from the SDK.')
-    ..addOption('nnbd',
-        allowed: NnbdMode.names,
-        defaultsTo: NnbdMode.strong.name,
-        help: 'Which set of non-nullable type features to use.')
     ..addOption('output-directory',
         aliases: ['output_directory'],
         defaultsTo: "logs",
@@ -408,7 +404,6 @@ has been specified on the command line.''')
     'compiler',
     'runtime',
     'timeout',
-    'nnbd',
     'sanitizer',
     'enable-asserts',
     'use-cfe',
@@ -628,14 +623,13 @@ has been specified on the command line.''')
     }
 
     var progress = Progress.find(data["progress"] as String);
-    var nnbdMode = NnbdMode.find(data["nnbd"] as String);
 
     void addConfiguration(Configuration innerConfiguration,
         [String? namedConfiguration]) {
       var configuration = TestConfiguration(
           configuration: innerConfiguration,
           progress: progress,
-          selectors: _expandSelectors(data, innerConfiguration.nnbdMode),
+          selectors: _expandSelectors(data),
           build: data["build"] as bool,
           testList: data["test-list-contents"] as List<String>?,
           repeat: int.parse(data["repeat"] as String),
@@ -784,7 +778,6 @@ has been specified on the command line.''')
                     mode,
                     runtime,
                     system,
-                    nnbdMode: nnbdMode,
                     sanitizer: sanitizer,
                     timeout: timeout,
                     enableAsserts: data['enable-asserts'] as bool,
@@ -819,7 +812,7 @@ has been specified on the command line.''')
   ///
   /// If no selectors are explicitly given, uses the default suite patterns.
   Map<String, RegExp> _expandSelectors(
-      Map<String, dynamic> configuration, NnbdMode nnbdMode) {
+      Map<String, dynamic> configuration) {
     var selectors = configuration['selectors'] as List<String>? ?? [];
 
     if (selectors.isEmpty || configuration['default-suites'] as bool) {
@@ -904,7 +897,7 @@ class OptionParseException implements Exception {
 /// given filter options.
 ///
 /// If any of the options `--system`, `--arch`, `--mode`, `--compiler`,
-/// `--nnbd`, or `--runtime` (or their abbreviations) are passed, then only
+/// or `--runtime` (or their abbreviations) are passed, then only
 /// configurations matching those are shown.
 void findConfigurations(Map<String, dynamic> options) {
   var testMatrix = TestMatrix.fromPath('tools/bots/test_matrix.json');
@@ -935,11 +928,6 @@ void findConfigurations(Map<String, dynamic> options) {
   var compilers = [...(options['compiler'] as List<String>).map(Compiler.find)];
   var runtimes = [...(options['runtime'] as List<String>).map(Runtime.find)];
 
-  NnbdMode? nnbdMode;
-  if (options.containsKey('nnbd')) {
-    nnbdMode = NnbdMode.find(options['nnbd'] as String);
-  }
-
   var names = SplayTreeSet<String>();
   for (var configuration in testMatrix.configurations) {
     if (system != null && configuration.system != system) continue;
@@ -954,7 +942,6 @@ void findConfigurations(Map<String, dynamic> options) {
     if (runtimes.isNotEmpty && !runtimes.contains(configuration.runtime)) {
       continue;
     }
-    if (nnbdMode != null && configuration.nnbdMode != nnbdMode) continue;
 
     names.add(configuration.name);
   }
@@ -965,7 +952,6 @@ void findConfigurations(Map<String, dynamic> options) {
     if (modes.isNotEmpty) "mode=$modes",
     if (compilers.isNotEmpty) "compiler=$compilers",
     if (runtimes.isNotEmpty) "runtime=$runtimes",
-    if (nnbdMode != null) "nnbd=$nnbdMode",
   ];
 
   if (filters.isEmpty) {

pkg/test_runner/lib/src/test_suite.dart

@@ -349,7 +349,6 @@ class VMTestSuite extends TestSuite {
       if (expectations.contains(Expectation.crash)) '--suppress-core-dump',
       if (experiments.isNotEmpty)
         '--enable-experiment=${experiments.join(",")}',
-      if (configuration.nnbdMode == NnbdMode.strong) '--sound-null-safety',
       ...configuration.standardOptions,
       ...configuration.vmOptions,
       test.name

pkg/test_runner/test/options_test.dart

@@ -18,7 +18,7 @@ void main() {
 void testDefaults() {
   // TODO(rnystrom): Test other options.
   var configuration = parseConfiguration([]);
-  Expect.equals(NnbdMode.strong, configuration.nnbdMode);
+  Expect.equals(Progress.line, configuration.progress);
 }
 
 void testOptions() {
@@ -28,15 +28,12 @@ void testOptions() {
   Expect.equals(Mode.debug, configurations[0].mode);
   Expect.equals(Mode.release, configurations[1].mode);
 
-  var configuration = parseConfiguration(['--nnbd=weak']);
-  Expect.equals(NnbdMode.weak, configuration.nnbdMode);
-
   // Filter invalid configurations when not passing a named configuration.
   configurations = parseConfigurations(['--arch=simarm', '--system=android']);
   Expect.isEmpty(configurations);
 
   // Special handling for *-options.
-  configuration = parseConfiguration([
+  var configuration = parseConfiguration([
     '--dart2js-options=a b c',
     '--vm-options=d e f',
     '--shared-options=g h i'
@@ -90,25 +87,15 @@ void testValidation() {
   expectValidationError(['--progress=compact,silent'],
       '"compact,silent" is not an allowed value for option "--progress".');
 
-  expectValidationError(['--nnbd=unknown'],
-      '"unknown" is not an allowed value for option "--nnbd".');
-  // Don't allow multiple.
-  expectValidationError(['--nnbd=weak,strong'],
-      '"weak,strong" is not an allowed value for option "--nnbd".');
-
   // Don't allow invalid named configurations.
   expectValidationError(['-ninvalid-vm-android-simarm'],
       'The named configuration "invalid-vm-android-simarm" is invalid.');
 }
 
 void testSelectors() {
-  // Default null safe suites.
-  for (var arguments in [
-    <String>[],
-    ['--nnbd=strong'],
-    ['-nvm-strong']
-  ]) {
-    var configuration = parseConfiguration(arguments);
+  // Default suites.
+  {
+    final configuration = parseConfiguration([]);
     Expect.setEquals({
       'samples',
       'standalone',
@@ -119,7 +106,7 @@ void testSelectors() {
       'lib',
       'kernel',
       'ffi',
-    }, configuration.selectors.keys, "suites for $arguments");
+    }, configuration.selectors.keys, "default suites");
   }
 
   // The test runner can run individual tests by being given the

pkg/test_runner/test/test_file_test.dart

@@ -7,7 +7,6 @@
 import 'dart:io';
 
 import 'package:expect/expect.dart';
-import 'package:test_runner/src/feature.dart';
 import 'package:test_runner/src/path.dart';
 import 'package:test_runner/src/static_error.dart';
 import 'package:test_runner/src/test_file.dart';
@@ -112,15 +111,13 @@ void testParseOtherOptions() {
   /\/ ddcOptions=ddc options
   /\/ OtherResources=other resources
   /\/ SharedObjects=shared objects
-  /\/ Requirements=nnbd nnbd-strong
   """);
   Expect.listEquals(["dart", "options"], file.dartOptions);
   Expect.listEquals(["shared", "options"], file.sharedOptions);
   Expect.listEquals(["dart2js", "options"], file.dart2jsOptions);
   Expect.listEquals(["dart2wasm", "options"], file.dart2wasmOptions);
   Expect.listEquals(["ddc", "options"], file.ddcOptions);
   Expect.listEquals(["other", "resources"], file.otherResources);
-  Expect.listEquals([Feature.nnbd, Feature.nnbdStrong], file.requirements);
 
   // Disallows multiple lines for some options.
   expectParseThrows("""
@@ -143,10 +140,6 @@ void testParseOtherOptions() {
   /\/ ddcOptions=first
   /\/ ddcOptions=second
   """);
-  expectParseThrows("""
-  /\/ Requirements=nnbd
-  /\/ Requirements=nnbd-strong
-  """);
 
   // Merges multiple lines for others.
   file = createTestFile(source: """

pkg/test_runner/test/test_matrix.json

@@ -7,8 +7,6 @@
       "options": {
         "compiler": "dartkp"
       }
-    },
-    "vm-legacy": {},
-    "vm-strong": {}
+    }
   }
-}
+}