Skip to content

Commit

Permalink
Add an override_platforms config field
Browse files Browse the repository at this point in the history
See #391
  • Loading branch information
nex3 committed Oct 17, 2017
1 parent 465927c commit 37df6ea
Show file tree
Hide file tree
Showing 11 changed files with 477 additions and 27 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 0.12.25

* Add a `override_platforms` configuration field which allows test platforms'
settings (such as browsers' executables) to be overridden by the user.

* Add a `define_platforms` configuration field which makes it possible to define
new platforms that use the same logic as existing ones but have different
settings.

## 0.12.24+8

* `spawnHybridUri()` now interprets relative URIs correctly in browser tests.
Expand Down
23 changes: 22 additions & 1 deletion doc/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ tags:
* [Configuring Platforms](#configuring-platforms)
* [`on_os`](#on_os)
* [`on_platform`](#on_platform)
* [`define_platform`](#define_platform)
* [`override_platforms`](#override_platforms)
* [`define_platforms`](#define_platforms)
* [Browser Settings](#browser-settings)
* [`executable`](#executable)
* [Configuration Presets](#configuration-presets)
Expand Down Expand Up @@ -570,6 +571,26 @@ when running on a particular operating system, use [`on_os`](#on_os) instead.

This field counts as [test configuration](#test-configuration).

### `override_platforms`

This field allows you to customize the settings for built-in test platforms. It
takes a map from platform identifiers to settings for those platforms. For example:

```yaml
override_platforms:
chrome:
# The settings to override for this platform.
settings:
executable: chromium
```

This tells the test runner to use the `chromium` executable for Chrome tests. It
calls that executable with the same logic and flags it normally uses for Chrome.

Each platform can define exactly which settings it supports. All browsers
support [the same settings](#browser-settings), but the VM doesn't support any
settings and so can't be overridden.

### `define_platforms`

You can define new platforms in terms of old ones using the `define_platforms`
Expand Down
3 changes: 3 additions & 0 deletions lib/src/runner/browser/platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ class BrowserPlatform extends PlatformPlugin
ExecutableSettings parsePlatformSettings(YamlMap settings) =>
new ExecutableSettings.parse(settings);

ExecutableSettings mergePlatformSettings(ExecutableSettings settings1, ExecutableSettings settings2) =>
settings1.merge(settings2);

void customizePlatform(TestPlatform platform, ExecutableSettings settings) {
var oldSettings =
_browserSettings[platform] ?? _browserSettings[platform.root];
Expand Down
24 changes: 24 additions & 0 deletions lib/src/runner/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:boolean_selector/boolean_selector.dart';
import 'package:collection/collection.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;
import 'package:source_span/source_span.dart';

import '../backend/platform_selector.dart';
import '../backend/test_platform.dart';
Expand All @@ -18,6 +19,7 @@ import 'configuration/args.dart' as args;
import 'configuration/custom_platform.dart';
import 'configuration/load.dart';
import 'configuration/platform_selection.dart';
import 'configuration/platform_settings.dart';
import 'configuration/reporters.dart';
import 'configuration/suite.dart';
import 'configuration/values.dart';
Expand Down Expand Up @@ -172,6 +174,9 @@ class Configuration {

Set<String> _knownPresets;

/// Built-in platforms whose settings are overridden by the user.
final Map<String, PlatformSettings> overridePlatforms;

/// Platforms defined by the user in terms of existing platforms.
final Map<String, CustomPlatform> definePlatforms;

Expand Down Expand Up @@ -218,6 +223,7 @@ class Configuration {
Glob filename,
Iterable<String> chosenPresets,
Map<String, Configuration> presets,
Map<String, PlatformSettings> overridePlatforms,
Map<String, CustomPlatform> definePlatforms,
bool noRetry,

Expand Down Expand Up @@ -261,6 +267,7 @@ class Configuration {
filename: filename,
chosenPresets: chosenPresetSet,
presets: _withChosenPresets(presets, chosenPresetSet),
overridePlatforms: overridePlatforms,
definePlatforms: definePlatforms,
noRetry: noRetry,
suiteDefaults: new SuiteConfiguration(
Expand Down Expand Up @@ -316,6 +323,7 @@ class Configuration {
Glob filename,
Iterable<String> chosenPresets,
Map<String, Configuration> presets,
Map<String, PlatformSettings> overridePlatforms,
Map<String, CustomPlatform> definePlatforms,
bool noRetry,
SuiteConfiguration suiteDefaults})
Expand All @@ -337,6 +345,7 @@ class Configuration {
chosenPresets =
new UnmodifiableSetView(chosenPresets?.toSet() ?? new Set()),
presets = _map(presets),
overridePlatforms = _map(overridePlatforms),
definePlatforms = _map(definePlatforms),
_noRetry = noRetry,
suiteDefaults = pauseAfterLoad == true
Expand Down Expand Up @@ -399,6 +408,13 @@ class Configuration {
// We don't need to verify [customPlatforms] here because those platforms
// already need to be verified and resolved to create [allPlatforms].

for (var settings in overridePlatforms.values) {
if (!allPlatforms.any((platform) => platform.identifier == settings.identifier)) {
throw new SourceSpanFormatException(
'Unknown platform "${settings.identifier}".', settings.identifierSpan);
}
}

suiteDefaults.validatePlatforms(allPlatforms);
for (var config in presets.values) {
config.validatePlatforms(allPlatforms);
Expand Down Expand Up @@ -448,6 +464,12 @@ class Configuration {
filename: other._filename ?? _filename,
chosenPresets: chosenPresets.union(other.chosenPresets),
presets: _mergeConfigMaps(presets, other.presets),
overridePlatforms: mergeUnmodifiableMaps(
overridePlatforms, other.overridePlatforms,
value: (settings1, settings2) => new PlatformSettings(
settings1.identifier,
settings1.identifierSpan,
settings1.settings.toList()..addAll(settings2.settings))),
definePlatforms:
mergeUnmodifiableMaps(definePlatforms, other.definePlatforms),
noRetry: other._noRetry ?? _noRetry,
Expand Down Expand Up @@ -482,6 +504,7 @@ class Configuration {
Glob filename,
Iterable<String> chosenPresets,
Map<String, Configuration> presets,
Map<String, PlatformSettings> overridePlatforms,
Map<String, CustomPlatform> definePlatforms,
bool noRetry,

Expand Down Expand Up @@ -523,6 +546,7 @@ class Configuration {
filename: filename ?? _filename,
chosenPresets: chosenPresets ?? this.chosenPresets,
presets: presets ?? this.presets,
overridePlatforms: overridePlatforms ?? this.overridePlatforms,
definePlatforms: definePlatforms ?? this.definePlatforms,
noRetry: noRetry ?? _noRetry,
suiteDefaults: suiteDefaults.change(
Expand Down
34 changes: 33 additions & 1 deletion lib/src/runner/configuration/load.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import '../configuration.dart';
import '../configuration/suite.dart';
import 'custom_platform.dart';
import 'platform_selection.dart';
import 'platform_settings.dart';
import 'reporters.dart';

/// A regular expression matching a Dart identifier.
Expand Down Expand Up @@ -193,6 +194,7 @@ class _ConfigurationLoader {
_disallow("plain_names");
_disallow("platforms");
_disallow("add_presets");
_disallow("override_platforms");
return Configuration.empty;
}

Expand All @@ -215,13 +217,42 @@ class _ConfigurationLoader {
var chosenPresets = _getList("add_presets",
(presetNode) => _parseIdentifierLike(presetNode, "Preset name"));

var overridePlatforms = _loadOverridePlatforms();

return new Configuration(
pauseAfterLoad: pauseAfterLoad,
runSkipped: runSkipped,
reporter: reporter,
concurrency: concurrency,
platforms: platforms,
chosenPresets: chosenPresets);
chosenPresets: chosenPresets,
overridePlatforms: overridePlatforms);
}

/// Loads the `override_platforms` field.
Map<String, PlatformSettings> _loadOverridePlatforms() {
var platformsNode =
_getNode("override_platforms", "map", (value) => value is Map) as YamlMap;
if (platformsNode == null) return const {};

var platforms = <String, PlatformSettings>{};
platformsNode.nodes.forEach((identifierNode, valueNode) {
var identifier =
_parseIdentifierLike(identifierNode, "Platform identifier");

_validate(valueNode, "Platform definition must be a map.",
(value) => value is Map);
var map = valueNode as YamlMap;

var settings = _expect(map, "settings");
_validate(settings, "Must be a map.", (value) => value is Map);

platforms[identifier] = new PlatformSettings(
identifier,
identifierNode.span,
[settings as YamlMap]);
});
return platforms;
}

/// Loads runner configuration that's not allowed in the global configuration
Expand Down Expand Up @@ -316,6 +347,7 @@ class _ConfigurationLoader {
});
}

/// Loads the `define_platforms` field.
Map<String, CustomPlatform> _loadDefinePlatforms() {
var platformsNode =
_getNode("define_platforms", "map", (value) => value is Map) as YamlMap;
Expand Down
26 changes: 26 additions & 0 deletions lib/src/runner/configuration/platform_settings.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) 2017, 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 'package:source_span/source_span.dart';
import 'package:yaml/yaml.dart';

import '../plugin/customizable_platform.dart';

/// User-defined settings for a built-in test platform.
class PlatformSettings {
/// The identifier used to look up the platform being overridden.
final String identifier;

/// The location that [identifier] was defined in the configuration file.
final SourceSpan identifierSpan;

/// The user's settings for this platform.
///
/// This is a list of settings, from most global to most specific, that will
/// eventually be merged using [CustomizablePlatform.mergePlatformSettings].
final List<YamlMap> settings;

PlatformSettings(this.identifier, this.identifierSpan, List<YamlMap> settings)
: settings = new List.unmodifiable(settings);
}
28 changes: 21 additions & 7 deletions lib/src/runner/loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ class Loader {
/// their string identifiers.
final _platformsByIdentifier = <String, TestPlatform>{};

/// The user-provided settings for platforms.
final _platformSettings = <TestPlatform, YamlMap>{};
/// The user-provided settings for platforms, as a list of settings that will
/// be merged together using [CustomizablePlatform.mergePlatformSettings].
final _platformSettings = <TestPlatform, List<YamlMap>>{};

/// The user-provided settings for platforms.
final _parsedPlatformSettings = <TestPlatform, Object>{};
Expand Down Expand Up @@ -104,6 +105,8 @@ class Loader {
_registerCustomPlatforms();

_config.validatePlatforms(allPlatforms);

_registerPlatformOverrides();
}

/// Registers a [PlatformPlugin] for [platforms].
Expand Down Expand Up @@ -140,7 +143,19 @@ class Loader {
_platformCallbacks[platform] = _platformCallbacks[parent];
_platformsByIdentifier[platform.identifier] = platform;

_platformSettings[platform] = customPlatform.settings;
_platformSettings[platform] = [customPlatform.settings];
}
}

/// Registers users' platform settings from [Configuration.overridePlatforms].
void _registerPlatformOverrides() {
for (var settings in _config.overridePlatforms.values) {
var platform = _platformsByIdentifier[settings.identifier];

// This is officially validated in [Configuration.validatePlatforms].
assert(platform != null);

_platformSettings.putIfAbsent(platform, () => []).addAll(settings.settings);
}
}

Expand Down Expand Up @@ -258,7 +273,7 @@ class Loader {
if (settings == null) return;

if (plugin is CustomizablePlatform) {
parsed = plugin.parsePlatformSettings(settings);
parsed = settings.map(plugin.parsePlatformSettings).reduce(plugin.mergePlatformSettings);
plugin.customizePlatform(platform, parsed);
_parsedPlatformSettings[platform] = parsed;
} else {
Expand All @@ -268,9 +283,8 @@ class Loader {
identifier = platform.parent.identifier;
span = _config.definePlatforms[platform.identifier].parentSpan;
} else {
// TODO(nweiz): Get information from [_config.overridePlatforms] when
// that exists.
throw new UnimplementedError();
identifier = platform.identifier;
span = _config.overridePlatforms[platform.identifier].identifierSpan;
}

throw new SourceSpanFormatException(
Expand Down
9 changes: 9 additions & 0 deletions lib/src/runner/plugin/customizable_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ abstract class CustomizablePlatform<T> extends PlatformPlugin {
/// possible.
T parsePlatformSettings(YamlMap settings);

/// Merges [settings1] with [settings2] and returns a new settings object that
/// includes the configuration of both.
///
/// When the settings conflict, [settings2] should take priority.
///
/// This is used to merge global settings with local settings, or a custom
/// platform's settings with its parent's.
T mergePlatformSettings(T settings1, T settings2);

/// Defines user-provided [settings] for [platform].
///
/// The [platform] is a platform this plugin was declared to accept when
Expand Down
5 changes: 3 additions & 2 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,11 @@ List flatten(Iterable nested) {
/// creating a new map unnecessarily.
///
/// The return value *may or may not* be unmodifiable.
Map<K, V> mergeUnmodifiableMaps<K, V>(Map<K, V> map1, Map<K, V> map2) {
Map<K, V> mergeUnmodifiableMaps<K, V>(Map<K, V> map1, Map<K, V> map2,
{V value(V value1, V value2)}) {
if (map1.isEmpty) return map2;
if (map2.isEmpty) return map1;
return mergeMaps(map1, map2);
return mergeMaps(map1, map2, value: value);
}

/// Truncates [text] to fit within [maxLength].
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: test
version: 0.12.24+8
version: 0.12.25-dev
author: Dart Team <[email protected]>
description: A library for writing dart unit tests.
homepage: https://github.com/dart-lang/test
Expand Down
Loading

0 comments on commit 37df6ea

Please sign in to comment.