Skip to content

Commit e49ae54

Browse files
authored
Promote dart2wasm compiler support to the stable browser platform. (#2144)
Closes #2143 I ended up going on quite the cleanup adventure here � . But I think the result is much cleaner around how it handles --precompiled and the various compilers in the browser platform. We may want to adopt a similar cleanup in other platforms too. - Deletes the experimental wasm platform, moves it into the `chrome` platform (works on chrome stable now). - Refactors heavily how the browser platform works, abstracting away the logic for precompiled support, dart2js support, and dart2wasm support into separate classes which implement a new `CompilerSupport` interface. - Drops entirely support for `--pub-serve` mode. We could bring this back if necessary as another kind of `CompilerSupport` but I don't think it has any users (let me know if you feel different). - Each `CompilerSupport` instance now gets its own server, browser manager, secret, etc. This resolves complications around knowing which kinds of compiler a given request is asking for, which the previous model couldn't handle well. You will get multiple browser instances if multiple compilers are used. - ~Adds the "precompiled" compiler option. This isn't intended to be passed directly, it is inferred when passing the `--precompiled` argument. No other compiler options are allowed if it is used (although we could probably if desired?).~ - Dropped `isJS` and `isWasm` from `Runtime`, these no longer make sense. Added extensions to `Compiler` instead for convenience. Likely there is some additional consolidation that could happen here, as there is a fair bit of duplicate logic across some of the implementations. But I am not sure if it is really worth it or not.
1 parent 9fffb48 commit e49ae54

40 files changed

+741
-1378
lines changed

integration_tests/wasm/dart_test.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
platforms: [experimental-chrome-wasm]
1+
platforms: [chrome, firefox]
2+
compilers: [dart2wasm]

integration_tests/wasm/test/hello_world_test.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +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/sdk/issues/53493')
65
@TestOn('wasm')
76
// This retry is a regression test for https://github.com/dart-lang/test/issues/2006
87
@Retry(2)

pkgs/test/CHANGELOG.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
1-
## 1.24.10-wip
1+
## 1.25.0-wip
22

33
* Handle paths with leading `/` when spawning test isolates.
4+
* Add support for the `dart2wasm` compiler in chrome and firefox.
5+
* **BREAKING**: Remove the `experimental-chrome-wasm` platform, you can now use
6+
`-p chrome -c dart2wasm` instead.
7+
* Note that this has always been advertised as a change that would happen in a
8+
future non-breaking release.
9+
* **BREAKING**:Dropped support for `--pub-serve` which has long not been tested
10+
or supported.
11+
* We do not anticipate much if any actual breakage or existing usage of this
12+
feature, which is why we are making this change in a non-breaking release.
13+
* If you do require this feature, file an issue and we can look at adding it
14+
back.
415

516
## 1.24.9
617

pkgs/test/doc/configuration.md

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ tags:
4747
* [`concurrency`](#concurrency)
4848
* [`pause_after_load`](#pause_after_load)
4949
* [`run_skipped`](#run_skipped)
50-
* [`pub_serve`](#pub_serve)
5150
* [`reporter`](#reporter)
5251
* [`file_reporters`](#file_reporters)
5352
* [`fold_stack_frames`](#fold_stack_frames)
@@ -475,17 +474,6 @@ presets:
475474
paths: ["test/", "extra_test/"]
476475
```
477476

478-
### `pub_serve`
479-
480-
This field indicates that the test runner should run against a `pub serve`
481-
instance by default, and provides the port number for that instance. Note that
482-
if there is no `pub serve` instance running at that port, running the tests will
483-
fail by default.
484-
485-
```yaml
486-
pub_serve: 8081
487-
```
488-
489477
### `reporter`
490478

491479
This field indicates the default reporter to use. It may be set to "compact",
@@ -855,7 +843,6 @@ presets:
855843
browser:
856844
paths:
857845
- test/runner/browser
858-
- test/runner/pub_serve_test.dart
859846
```
860847

861848
The `presets` field counts as [test configuration](#test-configuration). It can

pkgs/test/lib/src/executable.dart

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import 'package:test_core/src/runner/hack_register_platform.dart'; // ignore: im
99

1010
import 'runner/browser/platform.dart';
1111
import 'runner/node/platform.dart';
12-
import 'runner/wasm/platform.dart';
1312

1413
Future<void> main(List<String> args) async {
1514
registerPlatformPlugin([Runtime.nodeJS], NodePlatform.new);
@@ -20,9 +19,6 @@ Future<void> main(List<String> args) async {
2019
Runtime.safari,
2120
Runtime.internetExplorer
2221
], BrowserPlatform.start);
23-
registerPlatformPlugin([
24-
Runtime.experimentalChromeWasm,
25-
], BrowserWasmPlatform.start);
2622

2723
await executable.main(args);
2824
}

pkgs/test/lib/src/runner/browser/browser_manager.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,7 @@ class BrowserManager {
155155
static Browser _newBrowser(Uri url, Runtime browser,
156156
ExecutableSettings settings, Configuration configuration) =>
157157
switch (browser.root) {
158-
Runtime.chrome ||
159-
Runtime.experimentalChromeWasm =>
160-
Chrome(url, configuration, settings: settings),
158+
Runtime.chrome => Chrome(url, configuration, settings: settings),
161159
Runtime.firefox => Firefox(url, settings: settings),
162160
Runtime.safari => Safari(url, settings: settings),
163161
Runtime.internetExplorer => InternetExplorer(url, settings: settings),
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
7+
import 'package:test_api/backend.dart' show StackTraceMapper, SuitePlatform;
8+
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
9+
import 'package:web_socket_channel/web_socket_channel.dart'; // ignore: implementation_imports
10+
11+
/// The shared interface for all compiler support libraries.
12+
abstract interface class CompilerSupport {
13+
/// The URL at which this compiler serves its tests.
14+
///
15+
/// Each compiler serves its tests under a different directory.
16+
Uri get serverUrl;
17+
18+
/// Compiles [dartPath] using [suiteConfig] for [platform].
19+
///
20+
/// [dartPath] is the path to the original `.dart` test suite, relative to the
21+
/// package root.
22+
Future<void> compileSuite(
23+
String dartPath, SuiteConfiguration suiteConfig, SuitePlatform platform);
24+
25+
/// Retrieves a stack trace mapper for [dartPath] if available.
26+
///
27+
/// [dartPath] is the path to the original `.dart` test suite, relative to the
28+
/// package root.
29+
StackTraceMapper? stackTraceMapperForPath(String dartPath);
30+
31+
/// Returns the eventual URI for the web socket, as well as the channel itself
32+
/// once the connection is established.
33+
(Uri uri, Future<WebSocketChannel> socket) get webSocket;
34+
35+
/// Closes down anything necessary for this implementation.
36+
Future<void> close();
37+
}
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:convert';
7+
import 'dart:io';
8+
9+
import 'package:http_multi_server/http_multi_server.dart';
10+
import 'package:path/path.dart' as p;
11+
import 'package:shelf/shelf.dart' as shelf;
12+
import 'package:shelf/shelf_io.dart' as shelf_io;
13+
import 'package:shelf_packages_handler/shelf_packages_handler.dart';
14+
import 'package:shelf_static/shelf_static.dart';
15+
import 'package:shelf_web_socket/shelf_web_socket.dart';
16+
import 'package:test_api/backend.dart' show StackTraceMapper, SuitePlatform;
17+
import 'package:test_core/src/runner/configuration.dart'; // ignore: implementation_imports
18+
import 'package:test_core/src/runner/dart2js_compiler_pool.dart'; // ignore: implementation_imports
19+
import 'package:test_core/src/runner/package_version.dart'; // ignore: implementation_imports
20+
import 'package:test_core/src/runner/suite.dart'; // ignore: implementation_imports
21+
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
22+
import 'package:test_core/src/util/package_config.dart'; // ignore: implementation_imports
23+
import 'package:test_core/src/util/stack_trace_mapper.dart'; // ignore: implementation_imports
24+
import 'package:web_socket_channel/web_socket_channel.dart';
25+
26+
import '../../../util/math.dart';
27+
import '../../../util/one_off_handler.dart';
28+
import '../../../util/package_map.dart';
29+
import '../../../util/path_handler.dart';
30+
import 'compiler_support.dart';
31+
32+
/// Support for Dart2Js compiled tests.
33+
class Dart2JsSupport implements CompilerSupport {
34+
/// Whether [close] has been called.
35+
bool _closed = false;
36+
37+
/// The temporary directory in which compiled JS is emitted.
38+
final _compiledDir = createTempDir();
39+
40+
/// A map from test suite paths to Futures that will complete once those
41+
/// suites are finished compiling.
42+
///
43+
/// This is used to make sure that a given test suite is only compiled once
44+
/// per run, rather than once per browser per run.
45+
final _compileFutures = <String, Future<void>>{};
46+
47+
/// The [Dart2JsCompilerPool] managing active instances of `dart2js`.
48+
final _compilerPool = Dart2JsCompilerPool();
49+
50+
/// The global test runner configuration.
51+
final Configuration _config;
52+
53+
/// The default template path.
54+
final String _defaultTemplatePath;
55+
56+
/// Mappers for Dartifying stack traces, indexed by test path.
57+
final _mappers = <String, StackTraceMapper>{};
58+
59+
/// A [PathHandler] used to serve test specific artifacts.
60+
final _pathHandler = PathHandler();
61+
62+
/// The root directory served statically by this server.
63+
final String _root;
64+
65+
/// Each compiler serves its tests under a different randomly-generated
66+
/// secret URI to ensure that other users on the same system can't snoop
67+
/// on data being served through this server, as well as distinguish tests
68+
/// from different compilers from each other.
69+
final String _secret = randomUrlSecret();
70+
71+
/// The underlying server.
72+
final shelf.Server _server;
73+
74+
/// A [OneOffHandler] for servicing WebSocket connections for
75+
/// [BrowserManager]s.
76+
///
77+
/// This is one-off because each [BrowserManager] can only connect to a single
78+
/// WebSocket.
79+
final _webSocketHandler = OneOffHandler();
80+
81+
@override
82+
Uri get serverUrl => _server.url.resolve('$_secret/');
83+
84+
Dart2JsSupport._(this._config, this._defaultTemplatePath, this._server,
85+
this._root, String faviconPath) {
86+
var cascade = shelf.Cascade()
87+
.add(_webSocketHandler.handler)
88+
.add(packagesDirHandler())
89+
.add(_pathHandler.handler)
90+
.add(createStaticHandler(_root))
91+
.add(_wrapperHandler);
92+
93+
var pipeline = const shelf.Pipeline()
94+
.addMiddleware(PathHandler.nestedIn(_secret))
95+
.addHandler(cascade.handler);
96+
97+
_server.mount(shelf.Cascade()
98+
.add(createFileHandler(faviconPath))
99+
.add(pipeline)
100+
.handler);
101+
}
102+
103+
static Future<Dart2JsSupport> start({
104+
required Configuration config,
105+
required String defaultTemplatePath,
106+
required String root,
107+
required String faviconPath,
108+
}) async {
109+
var server = shelf_io.IOServer(await HttpMultiServer.loopback(0));
110+
return Dart2JsSupport._(
111+
config, defaultTemplatePath, server, root, faviconPath);
112+
}
113+
114+
/// A handler that serves wrapper files used to bootstrap tests.
115+
shelf.Response _wrapperHandler(shelf.Request request) {
116+
var path = p.fromUri(request.url);
117+
118+
if (path.endsWith('.html')) {
119+
var test = p.setExtension(path, '.dart');
120+
var scriptBase = htmlEscape.convert(p.basename(test));
121+
var link = '<link rel="x-dart-test" href="$scriptBase">';
122+
var testName = htmlEscape.convert(test);
123+
var template = _config.customHtmlTemplatePath ?? _defaultTemplatePath;
124+
var contents = File(template).readAsStringSync();
125+
var processedContents = contents
126+
// Checked during loading phase that there is only one {{testScript}} placeholder.
127+
.replaceFirst('{{testScript}}', link)
128+
.replaceAll('{{testName}}', testName);
129+
return shelf.Response.ok(processedContents,
130+
headers: {'Content-Type': 'text/html'});
131+
}
132+
133+
return shelf.Response.notFound('Not found.');
134+
}
135+
136+
@override
137+
Future<void> compileSuite(
138+
String dartPath, SuiteConfiguration suiteConfig, SuitePlatform platform) {
139+
return _compileFutures.putIfAbsent(dartPath, () async {
140+
var dir = Directory(_compiledDir).createTempSync('test_').path;
141+
var jsPath = p.join(dir, '${p.basename(dartPath)}.browser_test.dart.js');
142+
var bootstrapContent = '''
143+
${suiteConfig.metadata.languageVersionComment ?? await rootPackageLanguageVersionComment}
144+
import "package:test/src/bootstrap/browser.dart";
145+
146+
import "${p.toUri(p.absolute(dartPath))}" as test;
147+
148+
void main() {
149+
internalBootstrapBrowserTest(() => test.main);
150+
}
151+
''';
152+
153+
await _compilerPool.compile(bootstrapContent, jsPath, suiteConfig);
154+
if (_closed) return;
155+
156+
var bootstrapUrl = '${p.toUri(p.relative(dartPath, from: _root)).path}'
157+
'.browser_test.dart';
158+
_pathHandler.add(bootstrapUrl, (request) {
159+
return shelf.Response.ok(bootstrapContent,
160+
headers: {'Content-Type': 'application/dart'});
161+
});
162+
163+
var jsUrl = '${p.toUri(p.relative(dartPath, from: _root)).path}'
164+
'.browser_test.dart.js';
165+
_pathHandler.add(jsUrl, (request) {
166+
return shelf.Response.ok(File(jsPath).readAsStringSync(),
167+
headers: {'Content-Type': 'application/javascript'});
168+
});
169+
170+
var mapUrl = '${p.toUri(p.relative(dartPath, from: _root)).path}'
171+
'.browser_test.dart.js.map';
172+
_pathHandler.add(mapUrl, (request) {
173+
return shelf.Response.ok(File('$jsPath.map').readAsStringSync(),
174+
headers: {'Content-Type': 'application/json'});
175+
});
176+
177+
if (suiteConfig.jsTrace) return;
178+
var mapPath = '$jsPath.map';
179+
_mappers[dartPath] = JSStackTraceMapper(File(mapPath).readAsStringSync(),
180+
mapUrl: p.toUri(mapPath),
181+
sdkRoot: Uri.parse('org-dartlang-sdk:///sdk'),
182+
packageMap: (await currentPackageConfig).toPackageMap());
183+
});
184+
}
185+
186+
@override
187+
Future<void> close() async {
188+
if (_closed) return;
189+
_closed = true;
190+
await Future.wait([
191+
Directory(_compiledDir).deleteWithRetry(),
192+
_compilerPool.close(),
193+
_server.close(),
194+
]);
195+
}
196+
197+
@override
198+
StackTraceMapper? stackTraceMapperForPath(String dartPath) =>
199+
_mappers[dartPath];
200+
201+
@override
202+
(Uri, Future<WebSocketChannel>) get webSocket {
203+
var completer = Completer<WebSocketChannel>.sync();
204+
var path = _webSocketHandler.create(webSocketHandler(completer.complete));
205+
var webSocketUrl = serverUrl.replace(scheme: 'ws').resolve(path);
206+
return (webSocketUrl, completer.future);
207+
}
208+
}

0 commit comments

Comments
 (0)