From bfc313427dd46417ca5fad5a1adee228383ec42f Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Tue, 23 Feb 2016 14:02:48 -0800
Subject: [PATCH] Warn when an unsupported platform is passed.
If the config file contains a top-level test_on field, the runner will
now warn if the runner is invoked with an unsupported platform.
R=kevmoo@google.com
Review URL: https://codereview.chromium.org//1715523003 .
---
README.md | 5 +
doc/package_config.md | 18 ++-
lib/src/backend/operating_system.dart | 17 ++-
lib/src/backend/platform_selector.dart | 4 +-
lib/src/runner.dart | 52 ++++++++
lib/src/utils.dart | 8 +-
pubspec.yaml | 2 +-
test/io.dart | 3 +
test/runner/configuration/top_level_test.dart | 123 ++++++++++++++----
test/runner/test_on_test.dart | 2 -
10 files changed, 188 insertions(+), 46 deletions(-)
diff --git a/README.md b/README.md
index 30157a8a..80c4654d 100644
--- a/README.md
+++ b/README.md
@@ -176,6 +176,11 @@ specifies exactly which platforms a test can run on. It can be as simple as the
name of a platform, or a more complex Dart-like boolean expression involving
these platform names.
+You can also declare that your entire package only works on certain platforms by
+adding a [`test_on` field][test_on] to your package config file.
+
+[test_on]: https://github.com/dart-lang/test/blob/master/doc/package_config.md#test_on
+
### Platform Selectors
Platform selectors use the [boolean selector syntax][] defined in the
diff --git a/doc/package_config.md b/doc/package_config.md
index 23db939b..bead8c79 100644
--- a/doc/package_config.md
+++ b/doc/package_config.md
@@ -111,10 +111,10 @@ tags:
### `test_on`
-This field declares which platforms a test supports. It's usually applied to
-[specific tags](#configuring-tags) rather than used at the top level. It takes a
-[platform selector][] and only allows a test to run on platforms that match the
-selector.
+This field declares which platforms a test supports. It takes a
+[platform selector][] and only allows tests to run on platforms that match the
+selector. It's often used with [specific tags](#configuring-tags) to ensure that
+certain features will only be tested on supported platforms.
[platform selector]: https://github.com/dart-lang/test/blob/master/README.md#platform-selectors
@@ -124,6 +124,16 @@ tags:
promises: {test_on: "browser && !ie"}
```
+The field can also be used at the top level of the configuration file to
+indicate that the entire package only supports a particular platform. If someone
+tries to run the tests on an unsupported platform, the runner will print a
+warning and skip that platform.
+
+```yaml
+# This package uses dart:io.
+test_on: vm
+```
+
## Runner Configuration
Unlike [test configuration](#test-configuration), runner configuration affects
diff --git a/lib/src/backend/operating_system.dart b/lib/src/backend/operating_system.dart
index 4ed251d3..610f3b58 100644
--- a/lib/src/backend/operating_system.dart
+++ b/lib/src/backend/operating_system.dart
@@ -9,25 +9,25 @@
/// running the test runner.
class OperatingSystem {
/// Microsoft Windows.
- static const windows = const OperatingSystem._("windows");
+ static const windows = const OperatingSystem._("Windows", "windows");
/// Mac OS X.
- static const macOS = const OperatingSystem._("mac-os");
+ static const macOS = const OperatingSystem._("OS X", "mac-os");
/// GNU/Linux.
- static const linux = const OperatingSystem._("linux");
+ static const linux = const OperatingSystem._("Linux", "linux");
/// Android.
///
/// Since this is the operating system the test runner is running on, this
/// won't be true when testing remotely on an Android browser.
- static const android = const OperatingSystem._("android");
+ static const android = const OperatingSystem._("Android", "android");
/// No operating system.
///
/// This is used when running in the browser, or if an unrecognized operating
/// system is used. It can't be referenced by name in platform selectors.
- static const none = const OperatingSystem._("none");
+ static const none = const OperatingSystem._("none", "none");
/// A list of all instances of [OperatingSystem] other than [none].
static const all = const [windows, macOS, linux, android];
@@ -52,13 +52,16 @@ class OperatingSystem {
}
}
- /// The name of the operating system.
+ /// The human-friendly of the operating system.
final String name;
+ /// The identifier used to look up the operating system.
+ final String identifier;
+
/// Whether this is a POSIX-ish operating system.
bool get isPosix => this != windows && this != none;
- const OperatingSystem._(this.name);
+ const OperatingSystem._(this.name, this.identifier);
String toString() => name;
}
diff --git a/lib/src/backend/platform_selector.dart b/lib/src/backend/platform_selector.dart
index af55257b..10969f75 100644
--- a/lib/src/backend/platform_selector.dart
+++ b/lib/src/backend/platform_selector.dart
@@ -12,7 +12,7 @@ import 'test_platform.dart';
final _validVariables =
new Set<String>.from(["posix", "dart-vm", "browser", "js", "blink"])
..addAll(TestPlatform.all.map((platform) => platform.identifier))
- ..addAll(OperatingSystem.all.map((os) => os.name));
+ ..addAll(OperatingSystem.all.map((os) => os.identifier));
/// An expression for selecting certain platforms, including operating systems
/// and browsers.
@@ -46,7 +46,7 @@ class PlatformSelector {
return _inner.evaluate((variable) {
if (variable == platform.identifier) return true;
- if (variable == os.name) return true;
+ if (variable == os.identifier) return true;
switch (variable) {
case "dart-vm": return platform.isDartVM;
case "browser": return platform.isBrowser;
diff --git a/lib/src/runner.dart b/lib/src/runner.dart
index 4da2710b..1eb6f2c7 100644
--- a/lib/src/runner.dart
+++ b/lib/src/runner.dart
@@ -9,6 +9,8 @@ import 'package:async/async.dart';
import 'backend/group.dart';
import 'backend/group_entry.dart';
+import 'backend/operating_system.dart';
+import 'backend/platform_selector.dart';
import 'backend/suite.dart';
import 'backend/test.dart';
import 'backend/test_platform.dart';
@@ -105,6 +107,8 @@ class Runner {
throw new StateError("run() may not be called on a closed Runner.");
}
+ _warnForUnsupportedPlatforms();
+
var suites = _loadSuites();
var success;
@@ -139,6 +143,54 @@ class Runner {
return success == true;
}
+ /// Emits a warning if the user is trying to run on a platform that's
+ /// unsupported for the entire package.
+ void _warnForUnsupportedPlatforms() {
+ if (_config.testOn == PlatformSelector.all) return;
+
+ var unsupportedPlatforms = _config.platforms.where((platform) {
+ return !_config.testOn.evaluate(platform, os: currentOS);
+ }).toList();
+ if (unsupportedPlatforms.isEmpty) return;
+
+ // Human-readable names for all unsupported platforms.
+ var unsupportedNames = [];
+
+ // If the user tried to run on one or moe unsupported browsers, figure out
+ // whether we should warn about the individual browsers or whether all
+ // browsers are unsupported.
+ var unsupportedBrowsers = unsupportedPlatforms
+ .where((platform) => platform.isBrowser);
+ if (unsupportedBrowsers.isNotEmpty) {
+ var supportsAnyBrowser = TestPlatform.all
+ .where((platform) => platform.isBrowser)
+ .any((platform) => _config.testOn.evaluate(platform));
+
+ if (supportsAnyBrowser) {
+ unsupportedNames.addAll(
+ unsupportedBrowsers.map((platform) => platform.name));
+ } else {
+ unsupportedNames.add("browsers");
+ }
+ }
+
+ // If the user tried to run on the VM and it's not supported, figure out if
+ // that's because of the current OS or whether the VM is unsupported.
+ if (unsupportedPlatforms.contains(TestPlatform.vm)) {
+ var supportsAnyOS = OperatingSystem.all.any((os) =>
+ _config.testOn.evaluate(TestPlatform.vm, os: os));
+
+ if (supportsAnyOS) {
+ unsupportedNames.add(currentOS.name);
+ } else {
+ unsupportedNames.add("the Dart VM");
+ }
+ }
+
+ warn("this package doesn't support running tests on " +
+ toSentence(unsupportedNames, conjunction: "or") + ".");
+ }
+
/// Closes the runner.
///
/// This stops any future test suites from running. It will wait for any
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index da3f9af4..6d54faf0 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -103,12 +103,14 @@ String indent(String str) =>
/// Returns a sentence fragment listing the elements of [iter].
///
/// This converts each element of [iter] to a string and separates them with
-/// commas and/or "and" where appropriate.
-String toSentence(Iterable iter) {
+/// commas and/or [conjunction] where appropriate. The [conjunction] defaults to
+/// "and".
+String toSentence(Iterable iter, {String conjunction}) {
if (iter.length == 1) return iter.first.toString();
+
var result = iter.take(iter.length - 1).join(", ");
if (iter.length > 2) result += ",";
- return "$result and ${iter.last}";
+ return "$result ${conjunction ?? 'and'} ${iter.last}";
}
/// Returns [name] if [number] is 1, or the plural of [name] otherwise.
diff --git a/pubspec.yaml b/pubspec.yaml
index a4e6781e..3574b5cf 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: test
-version: 0.12.11-dev
+version: 0.12.11
author: Dart Team <misc@dartlang.org>
description: A library for writing dart unit tests.
homepage: https://github.com/dart-lang/test
diff --git a/test/io.dart b/test/io.dart
index 1db00ed7..f7ead650 100644
--- a/test/io.dart
+++ b/test/io.dart
@@ -33,6 +33,9 @@ final String noSuchFileMessage = Platform.isWindows
final _servingRegExp =
new RegExp(r'^Serving myapp [a-z]+ on http://localhost:(\d+)$');
+/// An operating system name that's different than the current operating system.
+final otherOS = Platform.isWindows ? "mac-os" : "windows";
+
/// A future that will return the port of a pub serve instance run via
/// [runPubServe].
///
diff --git a/test/runner/configuration/top_level_test.dart b/test/runner/configuration/top_level_test.dart
index 62103dbd..0069cf87 100644
--- a/test/runner/configuration/top_level_test.dart
+++ b/test/runner/configuration/top_level_test.dart
@@ -10,6 +10,7 @@ import 'package:path/path.dart' as p;
import 'package:scheduled_test/descriptor.dart' as d;
import 'package:scheduled_test/scheduled_stream.dart';
import 'package:scheduled_test/scheduled_test.dart';
+import 'package:test/src/util/io.dart';
import '../../io.dart';
@@ -109,40 +110,108 @@ void main() {
test.shouldExit(0);
});
- test("runs tests on a platform that matches test_on", () {
- d.file("dart_test.yaml", JSON.encode({
- "test_on": "vm"
- })).create();
+ group("test_on", () {
+ test("runs tests on a platform matching platform", () {
+ d.file("dart_test.yaml", JSON.encode({
+ "test_on": "vm"
+ })).create();
- d.file("test.dart", """
- import 'package:test/test.dart';
+ d.file("test.dart", """
+ import 'package:test/test.dart';
- void main() {
- test("test", () {});
- }
- """).create();
+ void main() {
+ test("test", () {});
+ }
+ """).create();
- var test = runTest(["test.dart"]);
- test.stdout.expect(consumeThrough(contains('All tests passed!')));
- test.shouldExit(0);
- });
+ var test = runTest(["test.dart"]);
+ test.stdout.expect(consumeThrough(contains('All tests passed!')));
+ test.shouldExit(0);
+ });
- test("doesn't run tests on a platform that doesn't match test_on", () {
- d.file("dart_test.yaml", JSON.encode({
- "test_on": "chrome"
- })).create();
+ test("warns about the VM when no OSes are supported", () {
+ d.file("dart_test.yaml", JSON.encode({
+ "test_on": "chrome"
+ })).create();
- d.file("test.dart", """
- import 'package:test/test.dart';
+ d.file("test.dart", """
+ import 'package:test/test.dart';
- void main() {
- test("test", () {});
- }
- """).create();
+ void main() {
+ test("test", () {});
+ }
+ """).create();
+
+ var test = runTest(["test.dart"]);
+ test.stderr.expect(
+ "Warning: this package doesn't support running tests on the Dart "
+ "VM.");
+ test.stdout.expect(consumeThrough(contains('No tests ran.')));
+ test.shouldExit(0);
+ });
+
+ test("warns about the OS when some OSes are supported", () {
+ d.file("dart_test.yaml", JSON.encode({
+ "test_on": otherOS
+ })).create();
+
+ d.file("test.dart", """
+ import 'package:test/test.dart';
- var test = runTest(["test.dart"]);
- test.stdout.expect(consumeThrough(contains('No tests ran.')));
- test.shouldExit(0);
+ void main() {
+ test("test", () {});
+ }
+ """).create();
+
+ var test = runTest(["test.dart"]);
+ test.stderr.expect(
+ "Warning: this package doesn't support running tests on "
+ "${currentOS.name}.");
+ test.stdout.expect(consumeThrough(contains('No tests ran.')));
+ test.shouldExit(0);
+ });
+
+ test("warns about browsers in general when no browsers are supported", () {
+ d.file("dart_test.yaml", JSON.encode({
+ "test_on": "vm"
+ })).create();
+
+ d.file("test.dart", """
+ import 'package:test/test.dart';
+
+ void main() {
+ test("test", () {});
+ }
+ """).create();
+
+ var test = runTest(["-p", "chrome", "test.dart"]);
+ test.stderr.expect(
+ "Warning: this package doesn't support running tests on browsers.");
+ test.stdout.expect(consumeThrough(contains('No tests ran.')));
+ test.shouldExit(0);
+ });
+
+ test("warns about specific browsers when specific browsers are "
+ "supported", () {
+ d.file("dart_test.yaml", JSON.encode({
+ "test_on": "safari"
+ })).create();
+
+ d.file("test.dart", """
+ import 'package:test/test.dart';
+
+ void main() {
+ test("test", () {});
+ }
+ """).create();
+
+ var test = runTest(["-p", "chrome,firefox,phantomjs", "test.dart"]);
+ test.stderr.expect(
+ "Warning: this package doesn't support running tests on Chrome, "
+ "Firefox, or PhantomJS.");
+ test.stdout.expect(consumeThrough(contains('No tests ran.')));
+ test.shouldExit(0);
+ });
});
test("uses the specified reporter", () {
diff --git a/test/runner/test_on_test.dart b/test/runner/test_on_test.dart
index 01b321c0..3fa8608d 100644
--- a/test/runner/test_on_test.dart
+++ b/test/runner/test_on_test.dart
@@ -13,8 +13,6 @@ import 'package:test/src/util/io.dart';
import '../io.dart';
-final _otherOS = Platform.isWindows ? "mac-os" : "windows";
-
void main() {
useSandbox();
--
GitLab