Skip to content

Commit

Permalink
Use package: URIs for imports when possible
Browse files Browse the repository at this point in the history
Fixes #2142

When a compiler is reading imports it's possible for a relative file
import and absolute package import to the same library to get treated as
separate libraries. The VM works around this in the case of a `main`
under `lib/` by checking when the entrypoint, and only the entrypoint,
could be referenced with a `package:` URI so that further relative URIs
resolved from that point are consistently treated as `package:` URIs.

Use the same workaround for bootstrap files. Add an `absoluteUri`
utility in `test_core` which prefers `package:` URIs when feasible, and
falls back to absolute file URIs otherwise. When this initial import
uses a `package:` URI it will make the subsequent relative imports
correctly resolve as `package:` URIs.

Add `async` and `await` as necessary since we're currently using the
async APIs to read the package config.
  • Loading branch information
natebosch committed Dec 5, 2023
1 parent cd9eba9 commit 1d80449
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 24 deletions.
1 change: 0 additions & 1 deletion integration_tests/regression/lib/issue_2142/test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
@Skip('https://github.com/dart-lang/test/issues/2142')
library;

import 'package:test/test.dart';
Expand Down
2 changes: 2 additions & 0 deletions pkgs/test/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## 1.24.10-wip

* Handle paths with leading `/` when spawning test isolates.
* Fix running of tests defined under `lib/` with relative imports to other
libraries in the package.

## 1.24.9

Expand Down
4 changes: 2 additions & 2 deletions pkgs/test/lib/src/runner/browser/platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,9 @@ class BrowserPlatform extends PlatformPlugin
var jsPath = p.join(dir, '${p.basename(dartPath)}.browser_test.dart.js');
var bootstrapContent = '''
${suiteConfig.metadata.languageVersionComment ?? await rootPackageLanguageVersionComment}
import "package:test/src/bootstrap/browser.dart";
import 'package:test/src/bootstrap/browser.dart';
import "${p.toUri(p.absolute(dartPath))}" as test;
import '${await absoluteUri(dartPath)}' as test;
void main() {
internalBootstrapBrowserTest(() => test.main);
Expand Down
2 changes: 2 additions & 0 deletions pkgs/test_core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## 0.5.10-wip

* Handle paths with leading `/` when spawning test isolates.
* Fix running of tests defined under `lib/` with relative imports to other
libraries in the package.

## 0.5.9

Expand Down
38 changes: 17 additions & 21 deletions pkgs/test_core/lib/src/runner/vm/platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class VMPlatform extends PlatformPlugin {
// ignore: deprecated_member_use, Remove when SDK constraint is at 3.2.0
var isolateID = Service.getIsolateID(isolate!)!;

var libraryPath = _absolute(path).toString();
var libraryPath = (await absoluteUri(path)).toString();
var serverUri = info.serverUri!;
client = await vmServiceConnectUri(_wsUriFor(serverUri).toString());
var isolateNumber = int.parse(isolateID.split('/').last);
Expand Down Expand Up @@ -164,12 +164,6 @@ class VMPlatform extends PlatformPlugin {
_tempDir.deleteWithRetry(),
]));

Uri _absolute(String path) {
final uri = p.toUri(path);
if (uri.isAbsolute) return uri;
return _workingDirectory.resolveUri(uri);
}

/// Compiles [path] to a native executable and spawns it as a process.
///
/// Sets up a communication channel as well by passing command line arguments
Expand All @@ -191,11 +185,11 @@ class VMPlatform extends PlatformPlugin {
path,
suiteMetadata.languageVersionComment ??
await rootPackageLanguageVersionComment);
var output = File(p.setExtension(bootstrapPath, '.exe'));
var output = File(p.setExtension(await bootstrapPath, '.exe'));
var processResult = await Process.run(Platform.resolvedExecutable, [
'compile',
'exe',
bootstrapPath,
await bootstrapPath,
'--output',
output.path,
'--packages',
Expand Down Expand Up @@ -231,7 +225,7 @@ stderr: ${processResult.stderr}''');
Compiler.kernel => _spawnIsolateWithUri(
await _compileToKernel(path, suiteMetadata), message),
Compiler.source => _spawnIsolateWithUri(
_bootstrapIsolateTestFile(
await _bootstrapIsolateTestFile(
path,
suiteMetadata.languageVersionComment ??
await rootPackageLanguageVersionComment),
Expand All @@ -247,12 +241,13 @@ stderr: ${processResult.stderr}''');

/// Compiles [path] to kernel and returns the uri to the compiled dill.
Future<Uri> _compileToKernel(String path, Metadata suiteMetadata) async {
final response = await _compiler.compile(_absolute(path), suiteMetadata);
final response =
await _compiler.compile(await absoluteUri(path), suiteMetadata);
var compiledDill = response.kernelOutputUri?.toFilePath();
if (compiledDill == null || response.errorCount > 0) {
throw LoadException(path, response.compilerOutput ?? 'unknown error');
}
return _absolute(compiledDill);
return absoluteUri(compiledDill);
}

/// Runs [uri] in an isolate, passing [message].
Expand All @@ -263,9 +258,10 @@ stderr: ${processResult.stderr}''');

Future<Isolate> _spawnPrecompiledIsolate(String testPath, SendPort message,
String precompiledPath, Compiler compiler) async {
testPath = _absolute('${p.join(precompiledPath, testPath)}.vm_test.dart')
.path
.stripDriveLetterLeadingSlash;
testPath =
(await absoluteUri('${p.join(precompiledPath, testPath)}.vm_test.dart'))
.path
.stripDriveLetterLeadingSlash;
switch (compiler) {
case Compiler.kernel:
var dillTestpath =
Expand Down Expand Up @@ -300,15 +296,15 @@ stderr: ${processResult.stderr}''');
/// file.
///
/// Returns the [Uri] to the created file.
Uri _bootstrapIsolateTestFile(
String testPath, String languageVersionComment) {
Future<Uri> _bootstrapIsolateTestFile(
String testPath, String languageVersionComment) async {
var file = File(p.join(
_tempDir.path, p.setExtension(testPath, '.bootstrap.isolate.dart')));
if (!file.existsSync()) {
file
..createSync(recursive: true)
..writeAsStringSync(_bootstrapIsolateTestContents(
_absolute(testPath), languageVersionComment));
await absoluteUri(testPath), languageVersionComment));
}
return file.uri;
}
Expand All @@ -317,15 +313,15 @@ stderr: ${processResult.stderr}''');
/// contents to a temporary file.
///
/// Returns the path to the created file.
String _bootstrapNativeTestFile(
String testPath, String languageVersionComment) {
Future<String> _bootstrapNativeTestFile(
String testPath, String languageVersionComment) async {
var file = File(p.join(
_tempDir.path, p.setExtension(testPath, '.bootstrap.native.dart')));
if (!file.existsSync()) {
file
..createSync(recursive: true)
..writeAsStringSync(_bootstrapNativeTestContents(
_absolute(testPath), languageVersionComment));
await absoluteUri(testPath), languageVersionComment));
}
return file.path;
}
Expand Down
12 changes: 12 additions & 0 deletions pkgs/test_core/lib/src/util/package_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// 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 'dart:isolate';

import 'package:package_config/package_config.dart';
import 'package:path/path.dart' as p;

/// The [PackageConfig] parsed from the current isolates package config file.
final Future<PackageConfig> currentPackageConfig = () async {
Expand All @@ -18,3 +20,13 @@ final Future<Uri> packageConfigUri = () async {
}
return uri;
}();

/// Returns an `package:` URI for [path] if it is in a package, otherwise
/// returns an absolute file URI.
Future<Uri> absoluteUri(String path) async {
final uri = p.toUri(path);
final absoluteUri =
uri.isAbsolute ? uri : Directory.current.uri.resolveUri(uri);
final packageConfig = await currentPackageConfig;
return packageConfig.toPackageUri(absoluteUri) ?? absoluteUri;
}

0 comments on commit 1d80449

Please sign in to comment.