Skip to content

Commit bfd24ca

Browse files
authored
Store test platforms as strings in Configuration (flutter#692)
This will make it easier to define custom test platforms (both via flutter#391 and flutter#99) in the future. Because those platforms will be loaded based on the configuration, requiring knowledge of them to parse the user's platform arguments would produce an unresolvable circular dependency.
1 parent fa5620d commit bfd24ca

File tree

10 files changed

+45
-38
lines changed

10 files changed

+45
-38
lines changed

lib/src/runner.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,9 @@ class Runner {
128128
if (testOn == PlatformSelector.all) return;
129129

130130
var unsupportedPlatforms = _config.suiteDefaults.platforms
131-
.where((platform) => !testOn.evaluate(platform, os: currentOS))
131+
.map(TestPlatform.find)
132+
.where((platform) =>
133+
platform != null && !testOn.evaluate(platform, os: currentOS))
132134
.toList();
133135
if (unsupportedPlatforms.isEmpty) return;
134136

@@ -351,7 +353,7 @@ class Runner {
351353
/// Loads each suite in [suites] in order, pausing after load for platforms
352354
/// that support debugging.
353355
Future<bool> _loadThenPause(Stream<LoadSuite> suites) async {
354-
if (_config.suiteDefaults.platforms.contains(TestPlatform.vm)) {
356+
if (_config.suiteDefaults.platforms.contains(TestPlatform.vm.identifier)) {
355357
warn("Debugging is currently unsupported on the Dart VM.",
356358
color: _config.color);
357359
}

lib/src/runner/browser/platform.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ class BrowserPlatform extends PlatformPlugin {
194194
/// Throws an [ArgumentError] if [browser] isn't a browser platform.
195195
Future<RunnerSuite> load(
196196
String path, TestPlatform browser, SuiteConfiguration suiteConfig) async {
197-
assert(suiteConfig.platforms.contains(browser));
197+
assert(suiteConfig.platforms.contains(browser.identifier));
198198

199199
if (!browser.isBrowser) {
200200
throw new ArgumentError("$browser is not a browser.");

lib/src/runner/configuration.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import 'package:glob/glob.dart';
1010
import 'package:path/path.dart' as p;
1111

1212
import '../backend/platform_selector.dart';
13-
import '../backend/test_platform.dart';
1413
import '../frontend/timeout.dart';
1514
import '../util/io.dart';
1615
import 'configuration/args.dart' as args;
@@ -220,7 +219,7 @@ class Configuration {
220219
Iterable<String> dart2jsArgs,
221220
String precompiledPath,
222221
Iterable<Pattern> patterns,
223-
Iterable<TestPlatform> platforms,
222+
Iterable<String> platforms,
224223
BooleanSelector includeTags,
225224
BooleanSelector excludeTags,
226225
Map<BooleanSelector, SuiteConfiguration> tags,
@@ -467,7 +466,7 @@ class Configuration {
467466
Iterable<String> dart2jsArgs,
468467
String precompiledPath,
469468
Iterable<Pattern> patterns,
470-
Iterable<TestPlatform> platforms,
469+
Iterable<String> platforms,
471470
BooleanSelector includeTags,
472471
BooleanSelector excludeTags,
473472
Map<BooleanSelector, SuiteConfiguration> tags,

lib/src/runner/configuration/args.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,7 @@ class _Parser {
220220
totalShards: totalShards,
221221
timeout: _parseOption('timeout', (value) => new Timeout.parse(value)),
222222
patterns: patterns,
223-
platforms:
224-
(_ifParsed('platform') as List<String>)?.map(TestPlatform.find),
223+
platforms: _ifParsed('platform') as List<String>,
225224
runSkipped: _ifParsed('run-skipped'),
226225
chosenPresets: _ifParsed('preset') as List<String>,
227226
paths: _options.rest.isEmpty ? null : _options.rest,

lib/src/runner/configuration/load.dart

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -206,15 +206,13 @@ class _ConfigurationLoader {
206206

207207
var concurrency = _getInt("concurrency");
208208

209-
var allPlatformIdentifiers =
210-
TestPlatform.all.map((platform) => platform.identifier).toSet();
211209
var platforms = _getList("platforms", (platformNode) {
212-
_validate(platformNode, "Platforms must be strings.",
213-
(value) => value is String);
214-
_validate(platformNode, 'Unknown platform "${platformNode.value}".',
215-
allPlatformIdentifiers.contains);
216-
217-
return TestPlatform.find(platformNode.value);
210+
var name = _parseIdentifierLike(platformNode, "Platform name");
211+
if (!TestPlatform.all.any((platform) => platform.identifier == name)) {
212+
throw new SourceSpanFormatException(
213+
'Unknown platform "$name"', platformNode.span, _source);
214+
}
215+
return name;
218216
});
219217

220218
var chosenPresets = _getList("add_presets",

lib/src/runner/configuration/suite.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ class SuiteConfiguration {
5151
final Set<Pattern> patterns;
5252

5353
/// The set of platforms on which to run tests.
54-
List<TestPlatform> get platforms => _platforms ?? const [TestPlatform.vm];
55-
final List<TestPlatform> _platforms;
54+
List<String> get platforms => _platforms ?? const ["vm"];
55+
final List<String> _platforms;
5656

5757
/// Only run tests whose tags match this selector.
5858
///
@@ -124,7 +124,7 @@ class SuiteConfiguration {
124124
Iterable<String> dart2jsArgs,
125125
String precompiledPath,
126126
Iterable<Pattern> patterns,
127-
Iterable<TestPlatform> platforms,
127+
Iterable<String> platforms,
128128
BooleanSelector includeTags,
129129
BooleanSelector excludeTags,
130130
Map<BooleanSelector, SuiteConfiguration> tags,
@@ -172,7 +172,7 @@ class SuiteConfiguration {
172172
Iterable<String> dart2jsArgs,
173173
this.precompiledPath,
174174
Iterable<Pattern> patterns,
175-
Iterable<TestPlatform> platforms,
175+
Iterable<String> platforms,
176176
BooleanSelector includeTags,
177177
BooleanSelector excludeTags,
178178
Map<BooleanSelector, SuiteConfiguration> tags,
@@ -249,7 +249,7 @@ class SuiteConfiguration {
249249
Iterable<String> dart2jsArgs,
250250
String precompiledPath,
251251
Iterable<Pattern> patterns,
252-
Iterable<TestPlatform> platforms,
252+
Iterable<String> platforms,
253253
BooleanSelector includeTags,
254254
BooleanSelector excludeTags,
255255
Map<BooleanSelector, SuiteConfiguration> tags,

lib/src/runner/loader.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ class Loader {
137137
return;
138138
}
139139

140-
for (var platform in suiteConfig.platforms) {
140+
for (var platformName in suiteConfig.platforms) {
141+
var platform = TestPlatform.find(platformName);
142+
141143
if (!suiteConfig.metadata.testOn.evaluate(platform, os: currentOS)) {
142144
continue;
143145
}

test/runner/browser/loader_test.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import '../../utils.dart';
2121
Loader _loader;
2222

2323
/// A configuration that loads suites on Chrome.
24-
final _chrome = new SuiteConfiguration(platforms: [TestPlatform.chrome]);
24+
final _chrome =
25+
new SuiteConfiguration(platforms: [TestPlatform.chrome.identifier]);
2526

2627
void main() {
2728
setUp(() async {
@@ -124,8 +125,10 @@ Future main() {
124125
var suites = await _loader
125126
.loadFile(
126127
path,
127-
new SuiteConfiguration(
128-
platforms: [TestPlatform.vm, TestPlatform.chrome]))
128+
new SuiteConfiguration(platforms: [
129+
TestPlatform.vm.identifier,
130+
TestPlatform.chrome.identifier
131+
]))
129132
.asyncMap((loadSuite) => loadSuite.getSuite())
130133
.toList();
131134
expect(suites[0].platform, equals(TestPlatform.vm));

test/runner/configuration/suite_test.dart

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,33 +18,34 @@ void main() {
1818
expect(merged.jsTrace, isFalse);
1919
expect(merged.runSkipped, isFalse);
2020
expect(merged.precompiledPath, isNull);
21-
expect(merged.platforms, equals([TestPlatform.vm]));
21+
expect(merged.platforms, equals([TestPlatform.vm.identifier]));
2222
});
2323

2424
test("if only the old configuration's is defined, uses it", () {
2525
var merged = new SuiteConfiguration(
26-
jsTrace: true,
27-
runSkipped: true,
28-
precompiledPath: "/tmp/js",
29-
platforms: [TestPlatform.chrome]).merge(new SuiteConfiguration());
26+
jsTrace: true,
27+
runSkipped: true,
28+
precompiledPath: "/tmp/js",
29+
platforms: [TestPlatform.chrome.identifier])
30+
.merge(new SuiteConfiguration());
3031

3132
expect(merged.jsTrace, isTrue);
3233
expect(merged.runSkipped, isTrue);
3334
expect(merged.precompiledPath, equals("/tmp/js"));
34-
expect(merged.platforms, equals([TestPlatform.chrome]));
35+
expect(merged.platforms, equals([TestPlatform.chrome.identifier]));
3536
});
3637

3738
test("if only the new configuration's is defined, uses it", () {
3839
var merged = new SuiteConfiguration().merge(new SuiteConfiguration(
3940
jsTrace: true,
4041
runSkipped: true,
4142
precompiledPath: "/tmp/js",
42-
platforms: [TestPlatform.chrome]));
43+
platforms: [TestPlatform.chrome.identifier]));
4344

4445
expect(merged.jsTrace, isTrue);
4546
expect(merged.runSkipped, isTrue);
4647
expect(merged.precompiledPath, equals("/tmp/js"));
47-
expect(merged.platforms, equals([TestPlatform.chrome]));
48+
expect(merged.platforms, equals([TestPlatform.chrome.identifier]));
4849
});
4950

5051
test(
@@ -54,18 +55,18 @@ void main() {
5455
jsTrace: false,
5556
runSkipped: true,
5657
precompiledPath: "/tmp/js",
57-
platforms: [TestPlatform.chrome]);
58+
platforms: [TestPlatform.chrome.identifier]);
5859
var newer = new SuiteConfiguration(
5960
jsTrace: true,
6061
runSkipped: false,
6162
precompiledPath: "../js",
62-
platforms: [TestPlatform.dartium]);
63+
platforms: [TestPlatform.dartium.identifier]);
6364
var merged = older.merge(newer);
6465

6566
expect(merged.jsTrace, isTrue);
6667
expect(merged.runSkipped, isFalse);
6768
expect(merged.precompiledPath, equals("../js"));
68-
expect(merged.platforms, equals([TestPlatform.dartium]));
69+
expect(merged.platforms, equals([TestPlatform.dartium.identifier]));
6970
});
7071
});
7172

test/runner/configuration/top_level_error_test.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,8 @@ void main() {
281281
.create();
282282

283283
var test = await runTest(["test.dart"]);
284-
expect(test.stderr, containsInOrder(["Platforms must be strings", "^^"]));
284+
expect(test.stderr,
285+
containsInOrder(["Platform name must be a string", "^^"]));
285286
await test.shouldExit(exit_codes.data);
286287
});
287288

@@ -294,7 +295,9 @@ void main() {
294295
}))
295296
.create();
296297

297-
var test = await runTest(["test.dart"]);
298+
await d.dir("test").create();
299+
300+
var test = await runTest([]);
298301
expect(test.stderr, containsInOrder(['Unknown platform "foo"', "^^^^^"]));
299302
await test.shouldExit(exit_codes.data);
300303
});

0 commit comments

Comments
 (0)