From 7d898f2a841da52effd692ffbd00cc7cd70526a6 Mon Sep 17 00:00:00 2001
From: Vyacheslav Egorov <vegorov@google.com>
Date: Fri, 3 Nov 2023 13:39:39 +0000
Subject: [PATCH] [infra] Allow overriding named configuration options

When doing local testing of AOT configurations I need to
manually override use-elf property to shorten cycle time.

Currently this can only be done by editing test_matrix.json,
this CL adds a capability to achieve the same by defining
TEST_CONFIGURATION_OVERRIDES environment variable.

I also fix some incorrect checking code which never triggered before:
if you try (via CLI flag) to override an option which is already
defined by a selected named configuration you should get an error.

Change-Id: Ie9b7fa12bd441814ca662e7e820c4aa739471c22
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333842
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
---
 pkg/smith/lib/configuration.dart     | 22 ++++++++++++++++++++++
 pkg/test_runner/lib/src/options.dart | 12 +++++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/pkg/smith/lib/configuration.dart b/pkg/smith/lib/configuration.dart
index 13c1ef792b7..9555c50edad 100644
--- a/pkg/smith/lib/configuration.dart
+++ b/pkg/smith/lib/configuration.dart
@@ -1,6 +1,7 @@
 // Copyright (c) 2018, 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.
+import 'dart:convert';
 import 'dart:io';
 
 // READ ME! If you add a new field to this, make sure to add it to
@@ -122,6 +123,17 @@ class Configuration {
     }
     var optionsCopy = Map.of(optionsJson);
 
+    // Apply overrides from the global environment variable.
+    final configurationOverridesJson =
+        _platformEnvironment['TEST_CONFIGURATION_OVERRIDES'];
+    if (configurationOverridesJson != null) {
+      final optionsOverrides =
+          jsonDecode(configurationOverridesJson) as Map<String, dynamic>;
+      for (var e in optionsOverrides.entries) {
+        optionsCopy[e.key] = e.value;
+      }
+    }
+
     T? enumOption<T extends NamedEnum>(
         String option, List<String> allowed, T Function(String) parse) {
       // Look up the value from the words in the name.
@@ -1147,3 +1159,13 @@ abstract class NamedEnum {
   @override
   String toString() => name;
 }
+
+final Map<String, String> _platformEnvironment = () {
+  try {
+    return Platform.environment;
+  } catch (_) {
+    // We might be running in the browser where Platform.environment just
+    // throws.
+    return const <String, String>{};
+  }
+}();
diff --git a/pkg/test_runner/lib/src/options.dart b/pkg/test_runner/lib/src/options.dart
index 8e9176cc26a..8b38f7aa3f3 100644
--- a/pkg/test_runner/lib/src/options.dart
+++ b/pkg/test_runner/lib/src/options.dart
@@ -448,8 +448,8 @@ has been specified on the command line.''')
     'csp',
     'minified',
     'vm-options',
-    'dart2js_options',
-    'experiments',
+    'dart2js-options',
+    'enable-experiment',
     'babel',
     'builder-tag',
     'use-qemu'
@@ -486,12 +486,14 @@ has been specified on the command line.''')
 
     // If a named configuration was specified ensure no other options, which are
     // implied by the named configuration, were specified.
-    if (options['named-configuration'] is String) {
+    final namedConfigurations = options['named-configuration'];
+    if (namedConfigurations is List<String> && namedConfigurations.isNotEmpty) {
       for (var optionName in _namedConfigurationOptions) {
         if (results.wasParsed(optionName)) {
-          var namedConfig = options['named-configuration'];
+          var namedConfigs =
+              (options['named-configuration'] as List<String>).join(', ');
           _fail("Can't pass '--$optionName' since it is determined by the "
-              "named configuration '$namedConfig'.");
+              "named configuration: $namedConfigs.");
         }
       }
     }