Refactor testing:TestConfiguration to use smith:Configuration by inclusion.
Change-Id: I5c116ad082a24c25a07b9ceb6aaf8c9cbe3f11e3
Reviewed-on: https://dart-review.googlesource.com/68361
Reviewed-by: Bob Nystrom <rnystrom@google.com>
diff --git a/pkg/smith/lib/configuration.dart b/pkg/smith/lib/configuration.dart
index 0c79fd1..949efcb 100644
--- a/pkg/smith/lib/configuration.dart
+++ b/pkg/smith/lib/configuration.dart
@@ -201,6 +201,18 @@
return value as String;
}
+ List<String> stringListOption(String option) {
+ if (!optionsCopy.containsKey(option)) return null;
+
+ var value = optionsCopy.remove(option);
+ if (value == null) throw FormatException('Option "$option" was null.');
+ if (value is! List) {
+ throw FormatException('Option "$option" had value "$value", which is '
+ 'not a List.');
+ }
+ return new List<String>.from(value);
+ }
+
// Extract options from the name and map.
var architecture =
enumOption("architecture", Architecture.names, Architecture.find);
@@ -236,14 +248,13 @@
var configuration = Configuration(
name, architecture, compiler, mode, runtime, system,
builderTag: stringOption("builder-tag"),
- vmOptions: stringOption("vm-options"),
+ vmOptions: stringListOption("vm-options"),
timeout: intOption("timeout"),
enableAsserts: boolOption("enable-asserts"),
isChecked: boolOption("checked"),
isCsp: boolOption("csp"),
isHostChecked: boolOption("host-checked"),
isMinified: boolOption("minified"),
- isStrong: boolOption("strong"),
previewDart2: boolOption("preview-dart-2"),
useBlobs: boolOption("use-blobs"),
useDart2JSWithKernel: boolOption("dart2js-with-kernel"),
@@ -273,12 +284,11 @@
final System system;
- // TODO(rnystrom): Is this still needed?
final String builderTag;
- final String vmOptions;
+ final List<String> vmOptions;
- final int timeout;
+ int timeout;
final bool enableAsserts;
@@ -293,9 +303,6 @@
final bool isMinified;
// TODO(rnystrom): Remove this when Dart 1.0 is no longer supported.
- final bool isStrong;
-
- // TODO(rnystrom): Remove this when Dart 1.0 is no longer supported.
final bool previewDart2;
// TODO(rnystrom): What is this?
@@ -315,14 +322,13 @@
Configuration(this.name, this.architecture, this.compiler, this.mode,
this.runtime, this.system,
{String builderTag,
- String vmOptions,
+ List<String> vmOptions,
int timeout,
bool enableAsserts,
bool isChecked,
bool isCsp,
bool isHostChecked,
bool isMinified,
- bool isStrong,
bool previewDart2,
bool useBlobs,
bool useDart2JSWithKernel,
@@ -332,14 +338,13 @@
bool useHotReloadRollback,
bool useSdk})
: builderTag = builderTag ?? "",
- vmOptions = vmOptions ?? "",
- timeout = timeout ?? 0,
+ vmOptions = vmOptions ?? <String>[],
+ timeout = timeout,
enableAsserts = enableAsserts ?? false,
isChecked = isChecked ?? false,
isCsp = isCsp ?? false,
isHostChecked = isHostChecked ?? false,
isMinified = isMinified ?? false,
- isStrong = isStrong ?? false,
previewDart2 = previewDart2 ?? true,
useBlobs = useBlobs ?? false,
useDart2JSWithKernel = useDart2JSWithKernel ?? false,
@@ -365,7 +370,6 @@
isCsp == other.isCsp &&
isHostChecked == other.isHostChecked &&
isMinified == other.isMinified &&
- isStrong == other.isStrong &&
previewDart2 == other.previewDart2 &&
useBlobs == other.useBlobs &&
useDart2JSWithKernel == other.useDart2JSWithKernel &&
@@ -393,15 +397,14 @@
(isCsp ? 4 : 0) ^
(isHostChecked ? 8 : 0) ^
(isMinified ? 16 : 0) ^
- (isStrong ? 32 : 0) ^
- (previewDart2 ? 64 : 0) ^
- (useBlobs ? 128 : 0) ^
- (useDart2JSWithKernel ? 256 : 0) ^
- (useDart2JSOldFrontEnd ? 512 : 0) ^
- (useFastStartup ? 1024 : 0) ^
- (useHotReload ? 2048 : 0) ^
- (useHotReloadRollback ? 4096 : 0) ^
- (useSdk ? 8192 : 0);
+ (previewDart2 ? 32 : 0) ^
+ (useBlobs ? 64 : 0) ^
+ (useDart2JSWithKernel ? 128 : 0) ^
+ (useDart2JSOldFrontEnd ? 256 : 0) ^
+ (useFastStartup ? 512 : 0) ^
+ (useHotReload ? 1024 : 0) ^
+ (useHotReloadRollback ? 2048 : 0) ^
+ (useSdk ? 4096 : 0);
String toString() {
var buffer = new StringBuffer();
@@ -423,7 +426,6 @@
if (isCsp) fields.add("csp");
if (isHostChecked) fields.add("host-checked");
if (isMinified) fields.add("minified");
- if (isStrong) fields.add("strong");
if (previewDart2) fields.add("preview-dart-2");
if (useBlobs) fields.add("use-blobs");
if (useDart2JSWithKernel) fields.add("dart2js-with-kernel");
diff --git a/pkg/smith/test/configuration_test.dart b/pkg/smith/test/configuration_test.dart
index 56404d0..626e9864 100644
--- a/pkg/smith/test/configuration_test.dart
+++ b/pkg/smith/test/configuration_test.dart
@@ -221,7 +221,7 @@
Runtime.d8,
System.host,
builderTag: "the tag",
- vmOptions: "vm stuff",
+ vmOptions: ["vm stuff", "more vm stuff"],
enableAsserts: true,
isChecked: true,
isCsp: true,
diff --git a/tools/bots/test_matrix.json b/tools/bots/test_matrix.json
index f7693cf..a6db839 100644
--- a/tools/bots/test_matrix.json
+++ b/tools/bots/test_matrix.json
@@ -167,7 +167,7 @@
"runtime": "vm",
"mode": "release",
"use-sdk": true,
- "vm-options": "-DuserFastParser=true",
+ "vm-options": ["-DuserFastParser=true"],
"builder-tag": "analyzer_use_fasta"
}},
"vm-legacy-(linux|mac|win)-(debug|release)-(ia32|x64)": {
@@ -227,14 +227,14 @@
"options": {
"preview-dart-2": false,
"builder-tag": "optimization_counter_threshold",
- "vm-options": "--optimization-counter-threshold=5"
+ "vm-options": ["--optimization-counter-threshold=5"]
}},
"vm-legacy-optcounter-checked-linux-release-(ia32|x64)": {
"options": {
"preview-dart-2": false,
"checked": true,
"builder-tag": "optimization_counter_threshold",
- "vm-options": "--optimization-counter-threshold=5"
+ "vm-options": ["--optimization-counter-threshold=5"]
}},
"vm-legacy-reload-linux-(debug|release)-x64": {
"options": {
@@ -346,7 +346,7 @@
"dartkp-linux-release-x64": { },
"dartkp-linux-debug-x64": {
"options": {
- "vm-options": "no-enable-malloc-hooks"
+ "vm-options": ["no-enable-malloc-hooks"]
}},
"dartk-(linux|mac)-(debug|release)-x64": { },
"dartk-win-release-x64": { },
@@ -355,7 +355,7 @@
"dartk-optcounter-linux-release-x64": {
"options": {
"builder-tag": "optimization_counter_threshold",
- "vm-options": "--optimization-counter-threshold=5",
+ "vm-options": ["--optimization-counter-threshold=5"],
"preview-dart-2": false
}},
"dartk-legacy-linux-release-x64": {
diff --git a/tools/testing/dart/configuration.dart b/tools/testing/dart/configuration.dart
index 5df9fc2..e532f7d 100644
--- a/tools/testing/dart/configuration.dart
+++ b/tools/testing/dart/configuration.dart
@@ -22,27 +22,17 @@
/// executed on, etc.
class TestConfiguration {
TestConfiguration(
- {this.architecture,
- this.compiler,
- this.mode,
+ {this.configuration,
+ this.namedConfiguration,
this.progress,
- this.runtime,
- this.system,
this.selectors,
this.appendLogs,
this.batch,
this.batchDart2JS,
this.copyCoreDumps,
- this.hotReload,
- this.hotReloadRollback,
- this.isChecked,
- this.isHostChecked,
- this.isCsp,
- this.isMinified,
this.isVerbose,
this.listTests,
this.listStatusFiles,
- this.noPreviewDart2,
this.printTiming,
this.printReport,
this.reportInJson,
@@ -50,17 +40,10 @@
this.skipCompilation,
this.useAnalyzerCfe,
this.useAnalyzerFastaParser,
- this.useBlobs,
- this.useSdk,
- this.useFastStartup,
- this.useEnableAsserts,
- this.useDart2JSWithKernel,
- this.useDart2JSOldFrontend,
this.useKernelBytecode,
this.writeDebugLog,
this.writeTestOutcomeLog,
this.writeResultLog,
- this.namedConfiguration,
this.drtPath,
this.chromePath,
this.safariPath,
@@ -69,7 +52,6 @@
this.dartPrecompiledPath,
this.flutterPath,
this.taskCount,
- int timeout,
this.shardCount,
this.shard,
this.stepName,
@@ -78,26 +60,23 @@
this.testDriverErrorPort,
this.localIP,
this.dart2jsOptions,
- this.vmOptions,
String packages,
this.packageRoot,
this.suiteDirectory,
- this.builderTag,
this.outputDirectory,
this.reproducingArguments,
this.fastTestsOnly,
this.printPassingStdout})
- : _packages = packages,
- _timeout = timeout;
-
- final Architecture architecture;
- final Compiler compiler;
- final Mode mode;
- final Progress progress;
- final Runtime runtime;
- final System system;
+ : _packages = packages;
final Map<String, RegExp> selectors;
+ final Progress progress;
+ // The test configuration computed from the test options.
+ final Configuration configuration;
+ // The test configuration coming from the -n option. Merging
+ // these two configurations into one will be the focus of some
+ // usability work.
+ final Configuration namedConfiguration;
// Boolean flags.
@@ -106,16 +85,9 @@
final bool batchDart2JS;
final bool copyCoreDumps;
final bool fastTestsOnly;
- final bool hotReload;
- final bool hotReloadRollback;
- final bool isChecked;
- final bool isHostChecked;
- final bool isCsp;
- final bool isMinified;
final bool isVerbose;
final bool listTests;
final bool listStatusFiles;
- final bool noPreviewDart2;
final bool printTiming;
final bool printReport;
final bool reportInJson;
@@ -123,20 +95,32 @@
final bool skipCompilation;
final bool useAnalyzerCfe;
final bool useAnalyzerFastaParser;
- final bool useBlobs;
- final bool useSdk;
- final bool useFastStartup;
- final bool useEnableAsserts;
- final bool useDart2JSWithKernel;
- final bool useDart2JSOldFrontend;
final bool useKernelBytecode;
final bool writeDebugLog;
final bool writeTestOutcomeLog;
final bool writeResultLog;
final bool printPassingStdout;
- final Configuration
- namedConfiguration; // The test configuration containing all test options.
+ Architecture get architecture => configuration.architecture;
+ Compiler get compiler => configuration.compiler;
+ Mode get mode => configuration.mode;
+ Runtime get runtime => configuration.runtime;
+ System get system => configuration.system;
+
+ // Boolean getters
+ bool get hotReload => configuration.useHotReload;
+ bool get hotReloadRollback => configuration.useHotReloadRollback;
+ bool get isChecked => configuration.isChecked;
+ bool get isHostChecked => configuration.isHostChecked;
+ bool get isCsp => configuration.isCsp;
+ bool get isMinified => configuration.isMinified;
+ bool get noPreviewDart2 => !configuration.previewDart2;
+ bool get useBlobs => configuration.useBlobs;
+ bool get useSdk => configuration.useSdk;
+ bool get useFastStartup => configuration.useFastStartup;
+ bool get useEnableAsserts => configuration.enableAsserts;
+ bool get useDart2JSWithKernel => configuration.useDart2JSWithKernel;
+ bool get useDart2JSOldFrontend => configuration.useDart2JSOldFrontEnd;
// Various file paths.
@@ -162,7 +146,7 @@
final List<String> dart2jsOptions;
/// Extra VM options passed to the testing script.
- final List<String> vmOptions;
+ List<String> get vmOptions => configuration.vmOptions;
String _packages;
@@ -178,7 +162,7 @@
final String outputDirectory;
final String packageRoot;
final String suiteDirectory;
- final String builderTag;
+ String get builderTag => configuration.builderTag;
final List<String> reproducingArguments;
TestingServers _servers;
@@ -226,10 +210,11 @@
/// build/none_vm_release_x64
String get buildDirectory => system.outputDirectory + configurationDirectory;
- 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 timeout {
- if (_timeout == null) {
+ if (configuration.timeout == null) {
var isReload = hotReload || hotReloadRollback;
var compilerMulitiplier = compilerConfiguration.timeoutMultiplier;
@@ -239,10 +224,10 @@
isReload: isReload,
arch: architecture);
- _timeout = 60 * compilerMulitiplier * runtimeMultiplier;
+ configuration.timeout = 60 * compilerMulitiplier * runtimeMultiplier;
}
- return _timeout;
+ return configuration.timeout;
}
List<String> get standardOptions {
diff --git a/tools/testing/dart/launch_browser.dart b/tools/testing/dart/launch_browser.dart
index 54c676f..b4f598e 100644
--- a/tools/testing/dart/launch_browser.dart
+++ b/tools/testing/dart/launch_browser.dart
@@ -34,7 +34,9 @@
}
var runtime = Runtime.find(name);
- var configuration = new TestConfiguration(runtime: runtime);
+ var configuration = new TestConfiguration(
+ configuration: new Configuration(
+ "dummy configuration", null, null, null, runtime, null));
var executable = configuration.browserLocation;
var browser = new Browser.byRuntime(runtime, executable);
browser.start(arguments[1]);
diff --git a/tools/testing/dart/options.dart b/tools/testing/dart/options.dart
index 441301b..dd6d7bc 100644
--- a/tools/testing/dart/options.dart
+++ b/tools/testing/dart/options.dart
@@ -4,7 +4,7 @@
import 'dart:io';
-import 'package:smith/smith.dart' as smith;
+import 'package:smith/smith.dart';
import 'configuration.dart';
import 'path.dart';
@@ -638,29 +638,42 @@
mode,
architecture,
system);
- print(namedConfiguration);
+ print(namedConfiguration); // TODO(whesse): remove
+ var innerConfiguration = new Configuration(
+ namedConfiguration?.name ?? "custom configuration",
+ architecture,
+ compiler,
+ mode,
+ runtime,
+ system,
+ timeout: data["timeout"] as int,
+ enableAsserts: data["enable_asserts"] as bool,
+ useBlobs: data["use_blobs"] as bool,
+ useSdk: data["use_sdk"] as bool,
+ useFastStartup: data["fast_startup"] as bool,
+ useDart2JSWithKernel: data["dart2js_with_kernel"] as bool,
+ useDart2JSOldFrontEnd: data["dart2js_old_frontend"] as bool,
+ useHotReload: data["hot_reload"] as bool,
+ useHotReloadRollback: data["hot_reload_rollback"] as bool,
+ isChecked: data["checked"] as bool,
+ isHostChecked: data["host_checked"] as bool,
+ isCsp: data["csp"] as bool,
+ isMinified: data["minified"] as bool,
+ vmOptions: vmOptions,
+ builderTag: data["builder_tag"] as String,
+ previewDart2: !(data["no_preview_dart_2"] as bool));
var configuration = new TestConfiguration(
- architecture: architecture,
- compiler: compiler,
- mode: mode,
+ configuration: innerConfiguration,
+ namedConfiguration: namedConfiguration,
progress: Progress.find(data["progress"] as String),
- runtime: runtime,
- system: System.find(data["system"] as String),
selectors: selectors,
appendLogs: data["append_logs"] as bool,
batch: !(data["noBatch"] as bool),
batchDart2JS: data["dart2js_batch"] as bool,
copyCoreDumps: data["copy_coredumps"] as bool,
- hotReload: data["hot_reload"] as bool,
- hotReloadRollback: data["hot_reload_rollback"] as bool,
- isChecked: data["checked"] as bool,
- isHostChecked: data["host_checked"] as bool,
- isCsp: data["csp"] as bool,
- isMinified: data["minified"] as bool,
isVerbose: data["verbose"] as bool,
listTests: data["list"] as bool,
listStatusFiles: data["list_status_files"] as bool,
- noPreviewDart2: data["no_preview_dart_2"] as bool,
printTiming: data["time"] as bool,
printReport: data["report"] as bool,
reportInJson: data["report_in_json"] as bool,
@@ -669,17 +682,10 @@
useAnalyzerCfe: data["use_cfe"] as bool,
useAnalyzerFastaParser:
data["analyzer_use_fasta_parser"] as bool,
- useBlobs: data["use_blobs"] as bool,
- useSdk: data["use_sdk"] as bool,
- useFastStartup: data["fast_startup"] as bool,
- useEnableAsserts: data["enable_asserts"] as bool,
- useDart2JSWithKernel: data["dart2js_with_kernel"] as bool,
- useDart2JSOldFrontend: data["dart2js_old_frontend"] as bool,
useKernelBytecode: compiler == Compiler.dartkb,
writeDebugLog: data["write_debug_log"] as bool,
writeTestOutcomeLog: data["write_test_outcome_log"] as bool,
writeResultLog: data["write_result_log"] as bool,
- namedConfiguration: namedConfiguration,
drtPath: data["drt"] as String,
chromePath: data["chrome"] as String,
safariPath: data["safari"] as String,
@@ -688,7 +694,6 @@
dartPrecompiledPath: data["dart_precompiled"] as String,
flutterPath: data["flutter"] as String,
taskCount: data["tasks"] as int,
- timeout: data["timeout"] as int,
shardCount: data["shards"] as int,
shard: data["shard"] as int,
stepName: data["step_name"] as String,
@@ -698,11 +703,9 @@
testDriverErrorPort: data["test_driver_error_port"] as int,
localIP: data["local_ip"] as String,
dart2jsOptions: dart2jsOptions,
- vmOptions: vmOptions,
packages: data["packages"] as String,
packageRoot: data["package_root"] as String,
suiteDirectory: data["suite_dir"] as String,
- builderTag: data["builder_tag"] as String,
outputDirectory: data["output_directory"] as String,
reproducingArguments: _reproducingCommand(data),
fastTestsOnly: data["fast_tests"] as bool,
@@ -859,7 +862,7 @@
}
}
-smith.Configuration getNamedConfiguration(String template, Runtime runtime,
+Configuration getNamedConfiguration(String template, Runtime runtime,
Compiler compiler, Mode mode, Architecture architecture, System system) {
print(template);
if (template == null) return null;
@@ -880,8 +883,7 @@
template = template.replaceFirst(r"${system}", name);
}
- smith.TestMatrix testMatrix =
- smith.TestMatrix.fromPath("tools/bots/test_matrix.json");
+ TestMatrix testMatrix = TestMatrix.fromPath("tools/bots/test_matrix.json");
print("Expanded namedConfiguration name:$template");
return testMatrix.configurations
.singleWhere((c) => c.name == template, orElse: () => null);
