Skip to content

Store test platforms as strings in Configuration #692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 3, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/src/runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ class Runner {
if (testOn == PlatformSelector.all) return;

var unsupportedPlatforms = _config.suiteDefaults.platforms
.where((platform) => !testOn.evaluate(platform, os: currentOS))
.map(TestPlatform.find)
.where((platform) =>
platform != null && !testOn.evaluate(platform, os: currentOS))
.toList();
if (unsupportedPlatforms.isEmpty) return;

Expand Down Expand Up @@ -351,7 +353,7 @@ class Runner {
/// Loads each suite in [suites] in order, pausing after load for platforms
/// that support debugging.
Future<bool> _loadThenPause(Stream<LoadSuite> suites) async {
if (_config.suiteDefaults.platforms.contains(TestPlatform.vm)) {
if (_config.suiteDefaults.platforms.contains(TestPlatform.vm.identifier)) {
warn("Debugging is currently unsupported on the Dart VM.",
color: _config.color);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/runner/browser/platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class BrowserPlatform extends PlatformPlugin {
/// Throws an [ArgumentError] if [browser] isn't a browser platform.
Future<RunnerSuite> load(
String path, TestPlatform browser, SuiteConfiguration suiteConfig) async {
assert(suiteConfig.platforms.contains(browser));
assert(suiteConfig.platforms.contains(browser.identifier));

if (!browser.isBrowser) {
throw new ArgumentError("$browser is not a browser.");
Expand Down
5 changes: 2 additions & 3 deletions lib/src/runner/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;

import '../backend/platform_selector.dart';
import '../backend/test_platform.dart';
import '../frontend/timeout.dart';
import '../util/io.dart';
import 'configuration/args.dart' as args;
Expand Down Expand Up @@ -220,7 +219,7 @@ class Configuration {
Iterable<String> dart2jsArgs,
String precompiledPath,
Iterable<Pattern> patterns,
Iterable<TestPlatform> platforms,
Iterable<String> platforms,
BooleanSelector includeTags,
BooleanSelector excludeTags,
Map<BooleanSelector, SuiteConfiguration> tags,
Expand Down Expand Up @@ -467,7 +466,7 @@ class Configuration {
Iterable<String> dart2jsArgs,
String precompiledPath,
Iterable<Pattern> patterns,
Iterable<TestPlatform> platforms,
Iterable<String> platforms,
BooleanSelector includeTags,
BooleanSelector excludeTags,
Map<BooleanSelector, SuiteConfiguration> tags,
Expand Down
3 changes: 1 addition & 2 deletions lib/src/runner/configuration/args.dart
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,7 @@ class _Parser {
totalShards: totalShards,
timeout: _parseOption('timeout', (value) => new Timeout.parse(value)),
patterns: patterns,
platforms:
(_ifParsed('platform') as List<String>)?.map(TestPlatform.find),
platforms: _ifParsed('platform') as List<String>,
runSkipped: _ifParsed('run-skipped'),
chosenPresets: _ifParsed('preset') as List<String>,
paths: _options.rest.isEmpty ? null : _options.rest,
Expand Down
12 changes: 2 additions & 10 deletions lib/src/runner/configuration/load.dart
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,8 @@ class _ConfigurationLoader {

var concurrency = _getInt("concurrency");

var allPlatformIdentifiers =
TestPlatform.all.map((platform) => platform.identifier).toSet();
var platforms = _getList("platforms", (platformNode) {
_validate(platformNode, "Platforms must be strings.",
(value) => value is String);
_validate(platformNode, 'Unknown platform "${platformNode.value}".',
allPlatformIdentifiers.contains);

return TestPlatform.find(platformNode.value);
});
var platforms = _getList("platforms",
(platformNode) => _parseIdentifierLike(platformNode, "Platform name"));

var chosenPresets = _getList("add_presets",
(presetNode) => _parseIdentifierLike(presetNode, "Preset name"));
Expand Down
10 changes: 5 additions & 5 deletions lib/src/runner/configuration/suite.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ class SuiteConfiguration {
final Set<Pattern> patterns;

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

/// Only run tests whose tags match this selector.
///
Expand Down Expand Up @@ -124,7 +124,7 @@ class SuiteConfiguration {
Iterable<String> dart2jsArgs,
String precompiledPath,
Iterable<Pattern> patterns,
Iterable<TestPlatform> platforms,
Iterable<String> platforms,
BooleanSelector includeTags,
BooleanSelector excludeTags,
Map<BooleanSelector, SuiteConfiguration> tags,
Expand Down Expand Up @@ -172,7 +172,7 @@ class SuiteConfiguration {
Iterable<String> dart2jsArgs,
this.precompiledPath,
Iterable<Pattern> patterns,
Iterable<TestPlatform> platforms,
Iterable<String> platforms,
BooleanSelector includeTags,
BooleanSelector excludeTags,
Map<BooleanSelector, SuiteConfiguration> tags,
Expand Down Expand Up @@ -249,7 +249,7 @@ class SuiteConfiguration {
Iterable<String> dart2jsArgs,
String precompiledPath,
Iterable<Pattern> patterns,
Iterable<TestPlatform> platforms,
Iterable<String> platforms,
BooleanSelector includeTags,
BooleanSelector excludeTags,
Map<BooleanSelector, SuiteConfiguration> tags,
Expand Down
4 changes: 3 additions & 1 deletion lib/src/runner/loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ class Loader {
return;
}

for (var platform in suiteConfig.platforms) {
for (var platformName in suiteConfig.platforms) {
var platform = TestPlatform.find(platformName);

if (!suiteConfig.metadata.testOn.evaluate(platform, os: currentOS)) {
continue;
}
Expand Down
9 changes: 6 additions & 3 deletions test/runner/browser/loader_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import '../../utils.dart';
Loader _loader;

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

void main() {
setUp(() async {
Expand Down Expand Up @@ -124,8 +125,10 @@ Future main() {
var suites = await _loader
.loadFile(
path,
new SuiteConfiguration(
platforms: [TestPlatform.vm, TestPlatform.chrome]))
new SuiteConfiguration(platforms: [
TestPlatform.vm.identifier,
TestPlatform.chrome.identifier
]))
.asyncMap((loadSuite) => loadSuite.getSuite())
.toList();
expect(suites[0].platform, equals(TestPlatform.vm));
Expand Down
23 changes: 12 additions & 11 deletions test/runner/configuration/suite_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,34 @@ void main() {
expect(merged.jsTrace, isFalse);
expect(merged.runSkipped, isFalse);
expect(merged.precompiledPath, isNull);
expect(merged.platforms, equals([TestPlatform.vm]));
expect(merged.platforms, equals([TestPlatform.vm.identifier]));
});

test("if only the old configuration's is defined, uses it", () {
var merged = new SuiteConfiguration(
jsTrace: true,
runSkipped: true,
precompiledPath: "/tmp/js",
platforms: [TestPlatform.chrome]).merge(new SuiteConfiguration());
jsTrace: true,
runSkipped: true,
precompiledPath: "/tmp/js",
platforms: [TestPlatform.chrome.identifier])
.merge(new SuiteConfiguration());

expect(merged.jsTrace, isTrue);
expect(merged.runSkipped, isTrue);
expect(merged.precompiledPath, equals("/tmp/js"));
expect(merged.platforms, equals([TestPlatform.chrome]));
expect(merged.platforms, equals([TestPlatform.chrome.identifier]));
});

test("if only the new configuration's is defined, uses it", () {
var merged = new SuiteConfiguration().merge(new SuiteConfiguration(
jsTrace: true,
runSkipped: true,
precompiledPath: "/tmp/js",
platforms: [TestPlatform.chrome]));
platforms: [TestPlatform.chrome.identifier]));

expect(merged.jsTrace, isTrue);
expect(merged.runSkipped, isTrue);
expect(merged.precompiledPath, equals("/tmp/js"));
expect(merged.platforms, equals([TestPlatform.chrome]));
expect(merged.platforms, equals([TestPlatform.chrome.identifier]));
});

test(
Expand All @@ -54,18 +55,18 @@ void main() {
jsTrace: false,
runSkipped: true,
precompiledPath: "/tmp/js",
platforms: [TestPlatform.chrome]);
platforms: [TestPlatform.chrome.identifier]);
var newer = new SuiteConfiguration(
jsTrace: true,
runSkipped: false,
precompiledPath: "../js",
platforms: [TestPlatform.dartium]);
platforms: [TestPlatform.dartium.identifier]);
var merged = older.merge(newer);

expect(merged.jsTrace, isTrue);
expect(merged.runSkipped, isFalse);
expect(merged.precompiledPath, equals("../js"));
expect(merged.platforms, equals([TestPlatform.dartium]));
expect(merged.platforms, equals([TestPlatform.dartium.identifier]));
});
});

Expand Down
7 changes: 5 additions & 2 deletions test/runner/configuration/top_level_error_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,8 @@ void main() {
.create();

var test = await runTest(["test.dart"]);
expect(test.stderr, containsInOrder(["Platforms must be strings", "^^"]));
expect(test.stderr,
containsInOrder(["Platform name must be a string", "^^"]));
await test.shouldExit(exit_codes.data);
});

Expand All @@ -294,7 +295,9 @@ void main() {
}))
.create();

var test = await runTest(["test.dart"]);
await d.dir("test").create();

var test = await runTest([]);
expect(test.stderr, containsInOrder(['Unknown platform "foo"', "^^^^^"]));
await test.shouldExit(exit_codes.data);
});
Expand Down