Fix test_runner package unit tests
Fix test_runner's handling of tests passed as
relative native file paths to the test runner command.
Fix failing use of a special executable to test a crashing process.
Change-Id: I0837a0c2c4461883a604aa85083c3098dfbaae2e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/286821
Reviewed-by: Devon Carew <devoncarew@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: William Hesse <whesse@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/pkg/test_runner/lib/src/options.dart b/pkg/test_runner/lib/src/options.dart
index dddf4d1..6eb2542 100644
--- a/pkg/test_runner/lib/src/options.dart
+++ b/pkg/test_runner/lib/src/options.dart
@@ -281,11 +281,6 @@
help: '''Which set of non-nullable type features to use.
Allowed values are: legacy, weak, strong''')
- // TODO(rnystrom): This does not appear to be used. Remove?
- ..addOption('build-directory',
- aliases: ['build_directory'],
- help: 'The name of the build directory, where products are placed.',
- hide: true)
..addOption('output-directory',
aliases: ['output_directory'],
defaultsTo: "logs",
@@ -504,8 +499,8 @@
var allSuiteDirectories = [
...testSuiteDirectories,
- "tests/co19",
- "tests/co19_2",
+ Path('tests/co19'),
+ Path('tests/co19_2'),
];
var selectors = <String>[];
@@ -515,20 +510,24 @@
// the command line.
for (var suiteDirectory in allSuiteDirectories) {
var path = suiteDirectory.toString();
- if (selector.startsWith("$path/") || selector.startsWith("$path\\")) {
- selector = selector.substring(path.lastIndexOf("/") + 1);
+ final separator = Platform.pathSeparator;
+ if (separator != '/') {
+ selector = selector.replaceAll(separator, '/');
+ }
+ if (selector.startsWith('$path/')) {
+ selector = selector.substring(path.lastIndexOf('/') + 1);
// Remove the `src/` subdirectories from the co19 and co19_2
// directories that do not appear in the test names.
- if (selector.startsWith("co19")) {
- selector = selector.replaceFirst(RegExp("src[/\]"), "");
+ if (selector.startsWith('co19')) {
+ selector = selector.replaceFirst(RegExp('src/'), '');
}
break;
}
}
// If they tab complete to a single test, ignore the ".dart".
- if (selector.endsWith(".dart")) {
+ if (selector.endsWith('.dart')) {
selector = selector.substring(0, selector.length - 5);
}
diff --git a/pkg/test_runner/lib/src/path.dart b/pkg/test_runner/lib/src/path.dart
index 7357190..df2878f 100644
--- a/pkg/test_runner/lib/src/path.dart
+++ b/pkg/test_runner/lib/src/path.dart
@@ -246,7 +246,7 @@
String toNativePath() {
if (isEmpty) return '.';
- if (Platform.operatingSystem == 'windows') {
+ if (Platform.isWindows) {
var nativePath = _path;
// Drop '/' before a drive letter.
if (nativePath.length >= 3 &&
diff --git a/pkg/test_runner/test/options_test.dart b/pkg/test_runner/test/options_test.dart
index 39694ac..3084305 100644
--- a/pkg/test_runner/test/options_test.dart
+++ b/pkg/test_runner/test/options_test.dart
@@ -1,6 +1,8 @@
// Copyright (c) 2019, 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 'dart:io';
+
import 'package:expect/expect.dart';
import 'package:test_runner/src/configuration.dart';
@@ -140,6 +142,35 @@
'ffi',
}, configuration.selectors.keys, "suites for $arguments");
}
+
+ // The test runner can run individual tests by being given the
+ // complete relative path to the test file, relative to the
+ // root of the Dart source checkout. The test runner parses
+ // these paths, if they match a known test suite location,
+ // and turns them into a suite name followed by a test path and name.
+ final testPath = ['tests', 'language', 'subdir', 'some_test.dart']
+ .join(Platform.pathSeparator);
+ final co19Path = [
+ 'tests',
+ 'co19_2',
+ 'src',
+ 'subdir_1',
+ 'subdir_src',
+ 'some_co19_test'
+ ].join(Platform.pathSeparator);
+ final configuration = parseConfiguration(
+ // Test arguments that include two test file paths and two
+ // suite selectors, one with a pattern ending in .dart.
+ // The final .dart is removed both from file paths and selector
+ // patterns.
+ ['vm', 'language_2/a_legacy_test.dart', testPath, co19Path]);
+ Expect.equals(
+ 'subdir/some_test', configuration.selectors['language']?.pattern);
+ Expect.equals('subdir_1/subdir_src/some_co19_test',
+ configuration.selectors['co19_2']?.pattern);
+ Expect.equals('.?', configuration.selectors['vm']?.pattern);
+ Expect.equals(
+ 'a_legacy_test', configuration.selectors['language_2']?.pattern);
}
TestConfiguration parseConfiguration(List<String> arguments) {
diff --git a/pkg/test_runner/test/test_runner_test.dart b/pkg/test_runner/test/test_runner_test.dart
index d1f7034..c99618a 100644
--- a/pkg/test_runner/test/test_runner_test.dart
+++ b/pkg/test_runner/test/test_runner_test.dart
@@ -2,10 +2,6 @@
// 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.
-// TODO(rnystrom): This test is only run by the analyzer and front end
-// configurations, so nothing is actually *executing* it. It's likely broken.
-// We should either remove it or get it working again.
-
import "dart:async";
import "dart:io";
@@ -21,8 +17,7 @@
import "package:test_runner/src/test_progress.dart" as progress;
import "package:test_runner/src/test_suite.dart";
-final DEFAULT_TIMEOUT = 20;
-final LONG_TIMEOUT = 30;
+final DEFAULT_TIMEOUT = 10;
List<String> packageOptions() {
if (Platform.packageConfig != null) {
@@ -78,7 +73,7 @@
onTest(testCase);
}
- var testCaseCrash = _makeCrashTestCase("crash", [Expectation.crash]);
+ var testCaseCrash = _makeNormalTestCase("crash", [Expectation.crash]);
var testCasePass = _makeNormalTestCase("pass", [Expectation.pass]);
var testCaseFail = _makeNormalTestCase("fail", [Expectation.fail]);
var testCaseTimeout = _makeNormalTestCase("timeout", [Expectation.timeout]);
@@ -100,17 +95,6 @@
return _makeTestCase(name, DEFAULT_TIMEOUT, command, expectations);
}
- TestCase _makeCrashTestCase(String name, Iterable<Expectation> expectations) {
- var crashCommand = ProcessCommand(
- 'custom_crash', getProcessTestFileName(), ["0", "0", "1", "1"], {});
- // The crash test sometimes times out. Run it with a large timeout
- // to help diagnose the delay.
- // The test loads a new executable, which may sometimes take a long time.
- // It involves a wait on the VM event loop, and possible system
- // delays.
- return _makeTestCase(name, LONG_TIMEOUT, crashCommand, expectations);
- }
-
TestCase _makeTestCase(String name, timeout, Command command,
Iterable<Expectation> expectations) {
var configuration = OptionsParser().parse(['--timeout', '$timeout'])[0];
@@ -147,11 +131,13 @@
testProcessQueue();
} else {
switch (arguments[0]) {
- case 'pass':
- return;
+ case 'crash':
+ exit(253);
case 'fail-unexpected':
case 'fail':
throw "This test always fails, to test the test scripts.";
+ case 'pass':
+ return;
case 'timeout':
// This process should be killed by the test after DEFAULT_TIMEOUT
Timer(const Duration(hours: 42), () {});
@@ -161,18 +147,3 @@
}
}
}
-
-String getPlatformExecutableExtension() {
- var os = Platform.operatingSystem;
- if (os == 'windows') return '.exe';
- return ''; // Linux and Mac OS.
-}
-
-String getProcessTestFileName() {
- var extension = getPlatformExecutableExtension();
- var executable = Platform.executable;
- var dirIndex = executable.lastIndexOf('dart');
- var buffer = StringBuffer(executable.substring(0, dirIndex));
- buffer.write('process_test$extension');
- return buffer.toString();
-}
