Skip to content

Commit fb3b572

Browse files
authored
Use package: URIs for imports when possible (#2148)
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.
1 parent 03cc56e commit fb3b572

File tree

7 files changed

+39
-28
lines changed

7 files changed

+39
-28
lines changed

integration_tests/regression/lib/issue_2142/test.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
@Skip('https://github.com/dart-lang/test/issues/2142')
6-
library;
7-
85
import 'package:test/test.dart';
96
import 'import.dart';
107

pkgs/test/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
feature, which is why we are making this change in a non-breaking release.
1313
* If you do require this feature, file an issue and we can look at adding it
1414
back.
15+
* Fix running of tests defined under `lib/` with relative imports to other
16+
libraries in the package.
1517

1618
## 1.24.9
1719

pkgs/test/lib/src/runner/browser/compilers/dart2js.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ class Dart2JsSupport implements CompilerSupport {
141141
var jsPath = p.join(dir, '${p.basename(dartPath)}.browser_test.dart.js');
142142
var bootstrapContent = '''
143143
${suiteConfig.metadata.languageVersionComment ?? await rootPackageLanguageVersionComment}
144-
import "package:test/src/bootstrap/browser.dart";
144+
import 'package:test/src/bootstrap/browser.dart';
145145
146-
import "${p.toUri(p.absolute(dartPath))}" as test;
146+
import '${await absoluteUri(dartPath)}' as test;
147147
148148
void main() {
149149
internalBootstrapBrowserTest(() => test.main);

pkgs/test/lib/src/runner/browser/compilers/dart2wasm.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import 'package:test_core/src/runner/package_version.dart'; // ignore: implement
1919
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
2020
import 'package:test_core/src/runner/wasm_compiler_pool.dart'; // ignore: implementation_imports
2121
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
22+
import 'package:test_core/src/util/package_config.dart'; // ignore: implementation_imports
2223
import 'package:web_socket_channel/web_socket_channel.dart';
2324

2425
import '../../../util/math.dart';
@@ -156,9 +157,9 @@ class Dart2WasmSupport implements CompilerSupport {
156157

157158
var bootstrapContent = '''
158159
${suiteConfig.metadata.languageVersionComment ?? await rootPackageLanguageVersionComment}
159-
import "package:test/src/bootstrap/browser.dart";
160+
import 'package:test/src/bootstrap/browser.dart';
160161
161-
import "${p.toUri(p.absolute(dartPath))}" as test;
162+
import '${await absoluteUri(dartPath)}' as test;
162163
163164
void main() {
164165
internalBootstrapBrowserTest(() => test.main);

pkgs/test_core/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* **BREAKING**: Removed `Runtime.isJS` and `Runtime.isWasm`, as this is now
77
based on the compiler and not the runtime.
88
* **BREAKING**: Removed `Configuration.pubServeUrl` and support for it.
9+
* Fix running of tests defined under `lib/` with relative imports to other
10+
libraries in the package.
911

1012
## 0.5.9
1113

pkgs/test_core/lib/src/runner/vm/platform.dart

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ class VMPlatform extends PlatformPlugin {
4040
final _compiler = TestCompiler(
4141
p.join(p.current, '.dart_tool', 'test', 'incremental_kernel'));
4242
final _closeMemo = AsyncMemoizer<void>();
43-
final _workingDirectory = Directory.current.uri;
4443
final _tempDir = Directory.systemTemp.createTempSync('dart_test.vm.');
4544

4645
@override
@@ -121,7 +120,7 @@ class VMPlatform extends PlatformPlugin {
121120
// ignore: deprecated_member_use, Remove when SDK constraint is at 3.2.0
122121
var isolateID = Service.getIsolateID(isolate!)!;
123122

124-
var libraryPath = _absolute(path).toString();
123+
var libraryPath = (await absoluteUri(path)).toString();
125124
var serverUri = info.serverUri!;
126125
client = await vmServiceConnectUri(_wsUriFor(serverUri).toString());
127126
var isolateNumber = int.parse(isolateID.split('/').last);
@@ -164,12 +163,6 @@ class VMPlatform extends PlatformPlugin {
164163
_tempDir.deleteWithRetry(),
165164
]));
166165

167-
Uri _absolute(String path) {
168-
final uri = p.toUri(path);
169-
if (uri.isAbsolute) return uri;
170-
return _workingDirectory.resolveUri(uri);
171-
}
172-
173166
/// Compiles [path] to a native executable and spawns it as a process.
174167
///
175168
/// Sets up a communication channel as well by passing command line arguments
@@ -187,7 +180,7 @@ class VMPlatform extends PlatformPlugin {
187180

188181
/// Compiles [path] to a native executable using `dart compile exe`.
189182
Future<String> _compileToNative(String path, Metadata suiteMetadata) async {
190-
var bootstrapPath = _bootstrapNativeTestFile(
183+
var bootstrapPath = await _bootstrapNativeTestFile(
191184
path,
192185
suiteMetadata.languageVersionComment ??
193186
await rootPackageLanguageVersionComment);
@@ -228,7 +221,7 @@ stderr: ${processResult.stderr}''');
228221
Compiler.kernel => _spawnIsolateWithUri(
229222
await _compileToKernel(path, suiteMetadata), message),
230223
Compiler.source => _spawnIsolateWithUri(
231-
_bootstrapIsolateTestFile(
224+
await _bootstrapIsolateTestFile(
232225
path,
233226
suiteMetadata.languageVersionComment ??
234227
await rootPackageLanguageVersionComment),
@@ -244,12 +237,13 @@ stderr: ${processResult.stderr}''');
244237

245238
/// Compiles [path] to kernel and returns the uri to the compiled dill.
246239
Future<Uri> _compileToKernel(String path, Metadata suiteMetadata) async {
247-
final response = await _compiler.compile(_absolute(path), suiteMetadata);
240+
final response =
241+
await _compiler.compile(await absoluteUri(path), suiteMetadata);
248242
var compiledDill = response.kernelOutputUri?.toFilePath();
249243
if (compiledDill == null || response.errorCount > 0) {
250244
throw LoadException(path, response.compilerOutput ?? 'unknown error');
251245
}
252-
return _absolute(compiledDill);
246+
return absoluteUri(compiledDill);
253247
}
254248

255249
/// Runs [uri] in an isolate, passing [message].
@@ -260,9 +254,10 @@ stderr: ${processResult.stderr}''');
260254

261255
Future<Isolate> _spawnPrecompiledIsolate(String testPath, SendPort message,
262256
String precompiledPath, Compiler compiler) async {
263-
testPath = _absolute('${p.join(precompiledPath, testPath)}.vm_test.dart')
264-
.path
265-
.stripDriveLetterLeadingSlash;
257+
testPath =
258+
(await absoluteUri('${p.join(precompiledPath, testPath)}.vm_test.dart'))
259+
.path
260+
.stripDriveLetterLeadingSlash;
266261
switch (compiler) {
267262
case Compiler.kernel:
268263
var dillTestpath =
@@ -297,15 +292,15 @@ stderr: ${processResult.stderr}''');
297292
/// file.
298293
///
299294
/// Returns the [Uri] to the created file.
300-
Uri _bootstrapIsolateTestFile(
301-
String testPath, String languageVersionComment) {
295+
Future<Uri> _bootstrapIsolateTestFile(
296+
String testPath, String languageVersionComment) async {
302297
var file = File(p.join(
303298
_tempDir.path, p.setExtension(testPath, '.bootstrap.isolate.dart')));
304299
if (!file.existsSync()) {
305300
file
306301
..createSync(recursive: true)
307302
..writeAsStringSync(_bootstrapIsolateTestContents(
308-
_absolute(testPath), languageVersionComment));
303+
await absoluteUri(testPath), languageVersionComment));
309304
}
310305
return file.uri;
311306
}
@@ -314,15 +309,15 @@ stderr: ${processResult.stderr}''');
314309
/// contents to a temporary file.
315310
///
316311
/// Returns the path to the created file.
317-
String _bootstrapNativeTestFile(
318-
String testPath, String languageVersionComment) {
312+
Future<String> _bootstrapNativeTestFile(
313+
String testPath, String languageVersionComment) async {
319314
var file = File(p.join(
320315
_tempDir.path, p.setExtension(testPath, '.bootstrap.native.dart')));
321316
if (!file.existsSync()) {
322317
file
323318
..createSync(recursive: true)
324319
..writeAsStringSync(_bootstrapNativeTestContents(
325-
_absolute(testPath), languageVersionComment));
320+
await absoluteUri(testPath), languageVersionComment));
326321
}
327322
return file.path;
328323
}

pkgs/test_core/lib/src/util/package_config.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:io';
56
import 'dart:isolate';
67

78
import 'package:package_config/package_config.dart';
9+
import 'package:path/path.dart' as p;
810

911
/// The [PackageConfig] parsed from the current isolates package config file.
1012
final Future<PackageConfig> currentPackageConfig = () async {
@@ -18,3 +20,15 @@ final Future<Uri> packageConfigUri = () async {
1820
}
1921
return uri;
2022
}();
23+
24+
final _originalWorkingDirectory = Directory.current.uri;
25+
26+
/// Returns an `package:` URI for [path] if it is in a package, otherwise
27+
/// returns an absolute file URI.
28+
Future<Uri> absoluteUri(String path) async {
29+
final uri = p.toUri(path);
30+
final absoluteUri =
31+
uri.isAbsolute ? uri : _originalWorkingDirectory.resolveUri(uri);
32+
final packageConfig = await currentPackageConfig;
33+
return packageConfig.toPackageUri(absoluteUri) ?? absoluteUri;
34+
}

0 commit comments

Comments
 (0)