From 1d80449750e925e5e81f6b6be3d3a5aa9a91aa66 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Tue, 5 Dec 2023 19:58:53 +0000 Subject: [PATCH] Use package: URIs for imports when possible 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. --- .../regression/lib/issue_2142/test.dart | 1 - pkgs/test/CHANGELOG.md | 2 + .../test/lib/src/runner/browser/platform.dart | 4 +- pkgs/test_core/CHANGELOG.md | 2 + .../test_core/lib/src/runner/vm/platform.dart | 38 +++++++++---------- .../lib/src/util/package_config.dart | 12 ++++++ 6 files changed, 35 insertions(+), 24 deletions(-) diff --git a/integration_tests/regression/lib/issue_2142/test.dart b/integration_tests/regression/lib/issue_2142/test.dart index 92efa4c8a..f510c09ea 100644 --- a/integration_tests/regression/lib/issue_2142/test.dart +++ b/integration_tests/regression/lib/issue_2142/test.dart @@ -1,4 +1,3 @@ -@Skip('https://github.com/dart-lang/test/issues/2142') library; import 'package:test/test.dart'; diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index b5cb03e3d..07fe2e006 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -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 diff --git a/pkgs/test/lib/src/runner/browser/platform.dart b/pkgs/test/lib/src/runner/browser/platform.dart index dc28f65fc..a29fbbc79 100644 --- a/pkgs/test/lib/src/runner/browser/platform.dart +++ b/pkgs/test/lib/src/runner/browser/platform.dart @@ -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); diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index 808ff03fc..8f111d065 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -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 diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index 6a57c3176..436cd775d 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/pkgs/test_core/lib/src/runner/vm/platform.dart @@ -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); @@ -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 @@ -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', @@ -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), @@ -247,12 +241,13 @@ stderr: ${processResult.stderr}'''); /// Compiles [path] to kernel and returns the uri to the compiled dill. Future _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]. @@ -263,9 +258,10 @@ stderr: ${processResult.stderr}'''); Future _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 = @@ -300,15 +296,15 @@ stderr: ${processResult.stderr}'''); /// file. /// /// Returns the [Uri] to the created file. - Uri _bootstrapIsolateTestFile( - String testPath, String languageVersionComment) { + Future _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; } @@ -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 _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; } diff --git a/pkgs/test_core/lib/src/util/package_config.dart b/pkgs/test_core/lib/src/util/package_config.dart index d6067f3ca..8e2593f3a 100644 --- a/pkgs/test_core/lib/src/util/package_config.dart +++ b/pkgs/test_core/lib/src/util/package_config.dart @@ -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 currentPackageConfig = () async { @@ -18,3 +20,13 @@ final Future packageConfigUri = () async { } return uri; }(); + +/// Returns an `package:` URI for [path] if it is in a package, otherwise +/// returns an absolute file URI. +Future 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; +}