Fix spawnHybridUri for NNBD (#1369)
Should fix https://github.com/dart-lang/test/issues/1363
I simplified the logic a bunch here, pushing all resolution of the uri to the server side.
Language comments are also inserted into the hybrid bootstrap code to match the original file being bootstrapped.
diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md
index 410f4f1..ded0c39 100644
--- a/pkgs/test/CHANGELOG.md
+++ b/pkgs/test/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 1.16.0-nullsafety.9
+
+* Fix `spawnHybridUri` to respect language versioning of the spawned uri.
+
## 1.16.0-nullsafety.8
* Update SDK constraints to `>=2.12.0-0 <3.0.0` based on beta release
diff --git a/pkgs/test/lib/src/runner/browser/platform.dart b/pkgs/test/lib/src/runner/browser/platform.dart
index ee5924c..d0e88a0 100644
--- a/pkgs/test/lib/src/runner/browser/platform.dart
+++ b/pkgs/test/lib/src/runner/browser/platform.dart
@@ -7,7 +7,6 @@
import 'dart:async';
import 'dart:convert';
import 'dart:io';
-import 'dart:isolate';
import 'package:async/async.dart';
import 'package:http_multi_server/http_multi_server.dart';
@@ -32,6 +31,7 @@
import 'package:test_core/src/runner/runner_suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
+import 'package:test_core/src/util/package_config.dart'; // ignore: implementation_imports
import 'package:test_core/src/util/stack_trace_mapper.dart'; // ignore: implementation_imports
import 'package:web_socket_channel/web_socket_channel.dart';
import 'package:yaml/yaml.dart';
@@ -51,12 +51,13 @@
/// the working directory.
static Future<BrowserPlatform> start({String root}) async {
var server = shelf_io.IOServer(await HttpMultiServer.loopback(0));
+ var packageConfig = await currentPackageConfig;
return BrowserPlatform._(
server,
Configuration.current,
- p.fromUri(await Isolate.resolvePackageUri(
+ p.fromUri(packageConfig.resolve(
Uri.parse('package:test/src/runner/browser/static/favicon.ico'))),
- p.fromUri(await Isolate.resolvePackageUri(Uri.parse(
+ p.fromUri(packageConfig.resolve(Uri.parse(
'package:test/src/runner/browser/static/default.html.tpl'))),
root: root);
}
diff --git a/pkgs/test/lib/src/runner/node/platform.dart b/pkgs/test/lib/src/runner/node/platform.dart
index 4e9eecb..a677fa6 100644
--- a/pkgs/test/lib/src/runner/node/platform.dart
+++ b/pkgs/test/lib/src/runner/node/platform.dart
@@ -24,6 +24,7 @@
import 'package:test_core/src/runner/runner_suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
+import 'package:test_core/src/util/package_config.dart'; // ignore: implementation_imports
import 'package:test_core/src/util/stack_trace_mapper.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/application_exception.dart'; // ignore: implementation_imports
import 'package:test_core/src/runner/compiler_pool.dart'; // ignore: implementation_imports
diff --git a/pkgs/test/lib/src/util/package_map.dart b/pkgs/test/lib/src/util/package_map.dart
index 8874c99..fd39842 100644
--- a/pkgs/test/lib/src/util/package_map.dart
+++ b/pkgs/test/lib/src/util/package_map.dart
@@ -4,15 +4,8 @@
//
// @dart=2.7
-import 'dart:isolate';
-
import 'package:package_config/package_config.dart';
-/// The [PackageConfig] parsed from the current isolates package config file.
-final Future<PackageConfig> currentPackageConfig = () async {
- return loadPackageConfigUri(await Isolate.packageConfig);
-}();
-
/// Adds methods to convert to a package map on [PackageConfig].
extension PackageMap on PackageConfig {
/// A package map exactly matching the current package config
diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml
index d596383..abd328d 100644
--- a/pkgs/test/pubspec.yaml
+++ b/pkgs/test/pubspec.yaml
@@ -1,5 +1,5 @@
name: test
-version: 1.16.0-nullsafety.8
+version: 1.16.0-nullsafety.9
description: A full featured library for writing and running Dart tests.
homepage: https://github.com/dart-lang/test/blob/master/pkgs/test
@@ -31,8 +31,8 @@
webkit_inspection_protocol: ">=0.5.0 <0.8.0"
yaml: ^2.0.0
# Use an exact version until the test_api and test_core package are stable.
- test_api: 0.2.19-nullsafety.5
- test_core: 0.3.12-nullsafety.8
+ test_api: 0.2.19-nullsafety.6
+ test_core: 0.3.12-nullsafety.9
dev_dependencies:
fake_async: ^1.0.0
diff --git a/pkgs/test/test/runner/hybrid_test.dart b/pkgs/test/test/runner/hybrid_test.dart
index 1ec99f7..8bc7327 100644
--- a/pkgs/test/test/runner/hybrid_test.dart
+++ b/pkgs/test/test/runner/hybrid_test.dart
@@ -9,6 +9,7 @@
import 'dart:io';
import 'dart:isolate';
+import 'package:package_config/package_config.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:test_descriptor/test_descriptor.dart' as d;
@@ -396,6 +397,33 @@
]));
await test.shouldExit(0);
});
+
+ test('can opt out of null safety', () async {
+ expect(spawnHybridCode('''
+ // @dart=2.9
+ import "package:stream_channel/stream_channel.dart";
+
+ // Would cause an error in null safety mode.
+ int x;
+
+ void hybridMain(StreamChannel channel) {
+ channel.sink..add(1)..add(2)..add(3)..close();
+ }
+ ''').stream.toList(), completion(equals([1, 2, 3])));
+ });
+
+ test('opts in to null safety by default', () async {
+ expect(spawnHybridCode('''
+ import "package:stream_channel/stream_channel.dart";
+
+ // Use some null safety syntax
+ int? x;
+
+ void hybridMain(StreamChannel channel) {
+ channel.sink..add(1)..add(2)..add(3)..close();
+ }
+ ''').stream.toList(), completion(equals([1, 2, 3])));
+ });
});
}
@@ -586,4 +614,107 @@
expect(spawnHybridUri('non existent file').stream.first,
throwsA(TypeMatcher<Exception>()));
});
+
+ test('can opt out of nnbd via language version comments', () async {
+ await d.file('test.dart', '''
+ import "package:test/test.dart";
+
+ void main() {
+ test("hybrid emits numbers", () {
+ expect(spawnHybridUri("hybrid.dart").stream.toList(),
+ completion(equals([1, 2, 3])));
+ });
+ }
+ ''').create();
+
+ await d.file('hybrid.dart', '''
+ // @dart=2.9
+ import "package:stream_channel/stream_channel.dart";
+
+ // Would fail if null safety were enabled.
+ int x;
+
+ void hybridMain(StreamChannel channel) {
+ channel.sink..add(1)..add(2)..add(3)..close();
+ }
+ ''').create();
+
+ var test = await runTest(['test.dart', ...arguments]);
+ expect(test.stdout,
+ containsInOrder(['+0: hybrid emits numbers', '+1: All tests passed!']));
+ await test.shouldExit(0);
+ });
+
+ test('can opt in to nnbd via language version comments', () async {
+ await d.file('test.dart', '''
+ import "package:test/test.dart";
+
+ void main() {
+ test("hybrid emits numbers", () {
+ expect(spawnHybridUri("hybrid.dart").stream.toList(),
+ completion(equals([1, 2, 3])));
+ });
+ }
+ ''').create();
+
+ await d.file('hybrid.dart', '''
+ // @dart=2.12
+ import "package:stream_channel/stream_channel.dart";
+
+ // Use some null safety syntax to confirm we are opted in.
+ int? x;
+
+ void hybridMain(StreamChannel channel) {
+ channel.sink..add(1)..add(2)..add(3)..close();
+ }
+ ''').create();
+
+ var test = await runTest(['test.dart', ...arguments]);
+ expect(test.stdout,
+ containsInOrder(['+0: hybrid emits numbers', '+1: All tests passed!']));
+ await test.shouldExit(0);
+ });
+
+ test('the default language version is used for hybrid code', () async {
+ await d.file('test.dart', '''
+ import "package:test/test.dart";
+
+ void main() {
+ test("hybrid emits numbers", () {
+ expect(spawnHybridUri("hybrid.dart").stream.toList(),
+ completion(equals([1, 2, 3])));
+ });
+ }
+ ''').create();
+
+ await d.file('hybrid.dart', '''
+ import "package:stream_channel/stream_channel.dart";
+
+ // Would fail if null safety were enabled.
+ int x;
+
+ void hybridMain(StreamChannel channel) {
+ channel.sink..add(1)..add(2)..add(3)..close();
+ }
+ ''').create();
+
+ // Adds the sandbox dir as a new package to the existing config,
+ // opting it out.
+ var originalPackageConfig =
+ await loadPackageConfigUri(await Isolate.packageConfig);
+ var extraPackage = Package('_test', Uri.file('${d.sandbox}/'),
+ languageVersion: LanguageVersion(2, 9));
+ var newConfig = PackageConfig([
+ ...originalPackageConfig.packages,
+ extraPackage,
+ ], extraData: originalPackageConfig.extraData);
+ await d.dir('.dart_tool').create();
+ await savePackageConfig(newConfig, Directory(d.sandbox));
+
+ var test = await runTest(['test.dart', ...arguments],
+ packageConfig: p.join(d.sandbox, '.dart_tool', 'package_config.json'));
+ expect(test.stdout,
+ containsInOrder(['+0: hybrid emits numbers', '+1: All tests passed!']));
+ await test.shouldExit(0);
+ });
}
diff --git a/pkgs/test_api/CHANGELOG.md b/pkgs/test_api/CHANGELOG.md
index 1694a52..7370d4e 100644
--- a/pkgs/test_api/CHANGELOG.md
+++ b/pkgs/test_api/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.2.19-nullsafety.6
+
+* Fix `spawnHybridUri` to respect language versioning of the spawned uri.
+
## 0.2.19-nullsafety.5
* Update SDK constraints to `>=2.12.0-0 <3.0.0` based on beta release
diff --git a/pkgs/test_api/lib/src/frontend/spawn_hybrid.dart b/pkgs/test_api/lib/src/frontend/spawn_hybrid.dart
index 4dde7b7..102298e 100644
--- a/pkgs/test_api/lib/src/frontend/spawn_hybrid.dart
+++ b/pkgs/test_api/lib/src/frontend/spawn_hybrid.dart
@@ -6,11 +6,9 @@
import 'dart:convert';
import 'package:async/async.dart';
-import 'package:path/path.dart' as p;
import 'package:stream_channel/stream_channel.dart';
import '../../test_api.dart';
-import '../backend/invoker.dart';
import '../util/remote_exception.dart';
import '../utils.dart';
@@ -92,52 +90,13 @@
/// **Note**: If you use this API, be sure to add a dependency on the
/// **`stream_channel` package, since you're using its API as well!
StreamChannel spawnHybridUri(uri, {Object? message, bool stayAlive = false}) {
- Uri parsedUrl;
- if (uri is Uri) {
- parsedUrl = uri;
- } else if (uri is String) {
- parsedUrl = Uri.parse(uri);
- } else {
+ if (uri is String) {
+ // Ensure that it can be parsed as a uri.
+ Uri.parse(uri);
+ } else if (uri is! Uri) {
throw ArgumentError.value(uri, 'uri', 'must be a Uri or a String.');
}
-
- String absoluteUri;
- if (parsedUrl.scheme.isEmpty) {
- var isRootRelative = parsedUrl.path.startsWith('/');
-
- // If we're running in a browser context, the working directory is already
- // relative to the test file, whereas on the VM the working directory is the
- // root of the package.
- if (p.style == p.Style.url) {
- if (isRootRelative) {
- // A root-relative URL is interpreted as relative to the package root,
- // which means placing it beneath the URL secret.
- var secret = Uri.encodeComponent(Uri.base.pathSegments[0]);
- absoluteUri = p.absolute('/$secret$parsedUrl');
- print('Uri.base: ${Uri.base}');
- print('absoluteUri: ${absoluteUri}');
- } else {
- absoluteUri = p.absolute(parsedUrl.toString());
- }
- } else {
- if (isRootRelative) {
- // We assume that the current path is the package root. `pub run`
- // enforces this currently, but at some point it would probably be good
- // to pass in an explicit root.
- absoluteUri = p.url
- .join(p.toUri(p.current).toString(), parsedUrl.path.substring(1));
- } else {
- var suitePath = Invoker.current!.liveTest.suite.path!;
- absoluteUri = p.url.join(
- p.url.dirname(p.toUri(p.absolute(suitePath)).toString()),
- parsedUrl.toString());
- }
- }
- } else {
- absoluteUri = uri.toString();
- }
-
- return _spawn(absoluteUri, message, stayAlive: stayAlive);
+ return _spawn(uri.toString(), message, stayAlive: stayAlive);
}
/// Spawns a VM isolate that runs the given [dartCode], which is loaded as the
@@ -187,8 +146,7 @@
return _spawn(uri.toString(), message, stayAlive: stayAlive);
}
-/// Like [spawnHybridUri], but doesn't take [Uri] objects and doesn't handle
-/// relative URLs.
+/// Like [spawnHybridUri], but doesn't take [Uri] objects.
StreamChannel _spawn(String uri, Object? message, {bool stayAlive = false}) {
var channel = Zone.current[#test.runner.test_channel] as MultiChannel?;
if (channel == null) {
diff --git a/pkgs/test_api/pubspec.yaml b/pkgs/test_api/pubspec.yaml
index ebe1d24..ac62ee1 100644
--- a/pkgs/test_api/pubspec.yaml
+++ b/pkgs/test_api/pubspec.yaml
@@ -1,5 +1,5 @@
name: test_api
-version: 0.2.19-nullsafety.5
+version: 0.2.19-nullsafety.6
description: A library for writing Dart tests.
homepage: https://github.com/dart-lang/test/blob/master/pkgs/test_api
diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md
index aba6648..251b589 100644
--- a/pkgs/test_core/CHANGELOG.md
+++ b/pkgs/test_core/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.3.12-nullsafety.9
+
+* Fix `spawnHybridUri` to respect language versioning of the spawned uri.
+
## 0.3.12-nullsafety.8
* Fix a bug where the test runner could crash when printing the elapsed time.
diff --git a/pkgs/test_core/lib/src/runner/runner_test.dart b/pkgs/test_core/lib/src/runner/runner_test.dart
index 2093212..5f3b0f7 100644
--- a/pkgs/test_core/lib/src/runner/runner_test.dart
+++ b/pkgs/test_core/lib/src/runner/runner_test.dart
@@ -71,7 +71,7 @@
// When we kill the isolate that the test lives in, that will close
// this virtual channel and cause the spawned isolate to close as
// well.
- spawnHybridUri(message['url'] as String, message['message'])
+ spawnHybridUri(message['url'] as String, message['message'], suite)
.pipe(testChannel.virtualChannel(message['channel'] as int));
break;
}
diff --git a/pkgs/test_core/lib/src/runner/spawn_hybrid.dart b/pkgs/test_core/lib/src/runner/spawn_hybrid.dart
index 768146b..14b537c 100644
--- a/pkgs/test_core/lib/src/runner/spawn_hybrid.dart
+++ b/pkgs/test_core/lib/src/runner/spawn_hybrid.dart
@@ -1,16 +1,23 @@
// Copyright (c) 2016, 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.
+//
+// @dart=2.9
import 'dart:async';
+import 'dart:io';
import 'dart:isolate';
+import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:async/async.dart';
+import 'package:path/path.dart' as p;
import 'package:stream_channel/isolate_channel.dart';
import 'package:stream_channel/stream_channel.dart';
import '../util/dart.dart' as dart;
+import '../util/package_config.dart';
+import 'package:test_api/src/backend/suite.dart'; // ignore: implementation_imports
import 'package:test_api/src/util/remote_exception.dart'; // ignore: implementation_imports
/// Spawns a hybrid isolate from [url] with the given [message], and returns a
@@ -19,12 +26,23 @@
/// This connects the main isolate to the hybrid isolate, whereas
/// `lib/src/frontend/spawn_hybrid.dart` connects the test isolate to the main
/// isolate.
-StreamChannel spawnHybridUri(String url, Object? message) {
+///
+/// If [uri] is relative, it will be interpreted relative to the `file:` URL
+/// for [suite]. If it's root-relative (that is, if it begins with `/`) it will
+/// be interpreted relative to the root of the package (the directory that
+/// contains `pubspec.yaml`, *not* the `test/` directory). If it's a `package:`
+/// URL, it will be resolved using the current package's dependency
+/// constellation.
+StreamChannel /*!*/ spawnHybridUri(
+ String url, Object /*?*/ message, Suite suite) {
+ url = _normalizeUrl(url, suite);
return StreamChannelCompleter.fromFuture(() async {
var port = ReceivePort();
var onExitPort = ReceivePort();
try {
var code = '''
+ ${await _languageVersionCommentFor(url)}
+
import "package:test_core/src/runner/hybrid_listener.dart";
import "${url.replaceAll(r'$', '%24')}" as lib;
@@ -64,3 +82,75 @@
}
}());
}
+
+/// Normalizes [url] to an absolute url, or returns it as is if it has a
+/// scheme.
+///
+/// Follows the rules for relatives/absolute paths outlit
+String _normalizeUrl(String url, Suite suite) {
+ final parsedUri = Uri.parse(url);
+
+ if (parsedUri.scheme.isEmpty) {
+ var isRootRelative = parsedUri.path.startsWith('/');
+
+ if (isRootRelative) {
+ // We assume that the current path is the package root. `pub run`
+ // enforces this currently, but at some point it would probably be good
+ // to pass in an explicit root.
+ return p.url
+ .join(p.toUri(p.current).toString(), parsedUri.path.substring(1));
+ } else {
+ var suitePath = suite.path;
+ return p.url.join(
+ p.url.dirname(p.toUri(p.absolute(suitePath)).toString()),
+ parsedUri.toString());
+ }
+ } else {
+ return url;
+ }
+}
+
+/// Computes the a language version comment for the library at [uri].
+///
+/// If there is a language version comment in the file, that is returned.
+///
+/// Otherwise a comment representing the default version from the
+/// [currentPackageConfig] is returned.
+///
+/// If no default language version is known (data: uri for instance), then
+/// an empty string is returned.
+Future<String> _languageVersionCommentFor(String url) async {
+ var parsedUri = Uri.parse(url);
+
+ // Returns the explicit language version comment if one exists.
+ var result = parseString(
+ content: await _readUri(parsedUri),
+ path: parsedUri.path,
+ throwIfDiagnostics: false);
+ var languageVersionComment = result.unit.languageVersionToken?.value();
+ if (languageVersionComment != null) return languageVersionComment.toString();
+
+ // Returns the default language version for the package if one exists.
+ if (parsedUri.scheme.isEmpty || parsedUri.scheme == 'file') {
+ var packageConfig = await currentPackageConfig;
+ var package = packageConfig.packageOf(parsedUri);
+ var version = package?.languageVersion;
+ if (version != null) return '// @dart=${version}';
+ }
+
+ // Fall back on no language comment.
+ return '';
+}
+
+Future<String> _readUri(Uri uri) async {
+ switch (uri.scheme) {
+ case '':
+ case 'file':
+ return File.fromUri(uri).readAsString();
+ case 'data':
+ return uri.data.contentAsString();
+ default:
+ throw ArgumentError.value(uri, 'uri',
+ 'Only data and file uris (as well as relative paths) are supported');
+ }
+}
diff --git a/pkgs/test_core/lib/src/util/package_config.dart b/pkgs/test_core/lib/src/util/package_config.dart
new file mode 100644
index 0000000..b3743f8
--- /dev/null
+++ b/pkgs/test_core/lib/src/util/package_config.dart
@@ -0,0 +1,14 @@
+// Copyright (c) 2020, 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.
+//
+// @dart=2.7
+
+import 'dart:isolate';
+
+import 'package:package_config/package_config.dart';
+
+/// The [PackageConfig] parsed from the current isolates package config file.
+final Future<PackageConfig> currentPackageConfig = () async {
+ return loadPackageConfigUri(await Isolate.packageConfig);
+}();
diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml
index 6df853e..8c55b48 100644
--- a/pkgs/test_core/pubspec.yaml
+++ b/pkgs/test_core/pubspec.yaml
@@ -1,5 +1,5 @@
name: test_core
-version: 0.3.12-nullsafety.8
+version: 0.3.12-nullsafety.9
description: A basic library for writing tests and running them on the VM.
homepage: https://github.com/dart-lang/test/blob/master/pkgs/test_core
@@ -30,7 +30,7 @@
# matcher is tightly constrained by test_api
matcher: any
# Use an exact version until the test_api package is stable.
- test_api: 0.2.19-nullsafety.5
+ test_api: 0.2.19-nullsafety.6
dependency_overrides:
test_api: