Skip to content

Run frontend server tests and fix failures #1658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## 14.0.4-dev
- Port some `dwds` files to null safety.
- Fix failing `frontend_server_evaluate` tests.

## 14.0.3
- Make data types null safe.
Expand Down
3 changes: 2 additions & 1 deletion dwds/lib/src/debugging/location.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// @dart = 2.9

import 'package:async/async.dart';
import 'package:dwds/src/loaders/require.dart';
import 'package:path/path.dart' as p;
import 'package:source_maps/parser.dart';
import 'package:source_maps/source_maps.dart';
Expand Down Expand Up @@ -272,7 +273,7 @@ class Locations {
await globalLoadStrategy.sourceMapPathForModule(_entrypoint, module);
final sourceMapContents =
await _assetReader.sourceMapContents(sourceMapPath);
final scriptLocation = p.url.dirname('/$modulePath');
final scriptLocation = p.url.dirname('/${relativizePath(modulePath)}');
if (sourceMapContents == null) return result;
// This happens to be a [SingleMapping] today in DDC.
final mapping = parse(sourceMapContents);
Expand Down
3 changes: 2 additions & 1 deletion dwds/lib/src/handlers/dev_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:dwds/src/loaders/require.dart';
import 'package:logging/logging.dart';
import 'package:shelf/shelf.dart';
import 'package:sse/server/sse_handler.dart';
Expand Down Expand Up @@ -201,7 +202,7 @@ class DevHandler {
'localhost',
webkitDebugger,
executionContext,
appTab.url,
basePathForServerUri(appTab.url),
_assetReader,
_loadStrategy,
appConnection,
Expand Down
11 changes: 7 additions & 4 deletions dwds/lib/src/loaders/frontend_server_require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ class FrontendServerRequireStrategyProvider {
RequireStrategy _requireStrategy;

FrontendServerRequireStrategyProvider(this._configuration, this._assetReader,
this._digestsProvider, String basePath)
: _basePath = basePathForServerUri(basePath);
this._digestsProvider, this._basePath);

RequireStrategy get strategy => _requireStrategy ??= RequireStrategy(
_configuration,
Expand All @@ -37,8 +36,12 @@ class FrontendServerRequireStrategyProvider {
_assetReader,
);

String _removeBasePath(String path) =>
path.startsWith(_basePath) ? path.substring(_basePath.length) : null;
String _removeBasePath(String path) {
if (_basePath.isEmpty) return path;
// If path is a server path it might start with a '/'.
final base = path.startsWith('/') ? '/$_basePath' : _basePath;
return path.startsWith(base) ? path.substring(base.length) : path;
}

String _addBasePath(String serverPath) =>
_basePath == null || _basePath.isEmpty
Expand Down
9 changes: 3 additions & 6 deletions dwds/lib/src/loaders/require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@ import '../services/expression_compiler.dart';
/// Find the path we are serving from the url.
///
/// Example:
/// https://localhost/base/index.html => /base
/// https://localhost/base => /base
/// https://localhost/base/index.html => base
/// https://localhost/base => base
String basePathForServerUri(String url) {
if (url == null) return null;
final uri = Uri.parse(url);
var base = uri.path.endsWith('.html') ? p.dirname(uri.path) : uri.path;
if (base.isNotEmpty) {
base = base.startsWith('/') ? base : '/$base';
}
return base;
return base = base.startsWith('/') ? base.substring(1) : base;
}

String relativizePath(String path) =>
Expand Down
20 changes: 10 additions & 10 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class ChromeProxyService implements VmServiceInterface {
Completer<void> _compilerCompleter = Completer<void>();
Future<void> get isCompilerInitialized => _compilerCompleter.future;

/// The root URI at which we're serving.
final String uri;
/// The root at which we're serving.
final String root;

final RemoteDebugger remoteDebugger;
final ExecutionContext executionContext;
Expand Down Expand Up @@ -104,7 +104,7 @@ class ChromeProxyService implements VmServiceInterface {

ChromeProxyService._(
this._vm,
this.uri,
this.root,
this._assetReader,
this.remoteDebugger,
this._modules,
Expand All @@ -120,14 +120,14 @@ class ChromeProxyService implements VmServiceInterface {
appInspectorProvider,
_locations,
_skipLists,
uri,
root,
);
_debuggerCompleter.complete(debugger);
}

static Future<ChromeProxyService> create(
RemoteDebugger remoteDebugger,
String tabUrl,
String root,
AssetReader assetReader,
LoadStrategy loadStrategy,
AppConnection appConnection,
Expand All @@ -150,12 +150,12 @@ class ChromeProxyService implements VmServiceInterface {
pid: -1,
);

final modules = Modules(tabUrl);
final locations = Locations(assetReader, modules, tabUrl);
final modules = Modules(root);
final locations = Locations(assetReader, modules, root);
final skipLists = SkipLists();
final service = ChromeProxyService._(
vm,
tabUrl,
root,
assetReader,
remoteDebugger,
modules,
Expand Down Expand Up @@ -229,7 +229,7 @@ class ChromeProxyService implements VmServiceInterface {
remoteDebugger,
_assetReader,
_locations,
uri,
root,
debugger,
executionContext,
sdkConfiguration,
Expand Down Expand Up @@ -352,7 +352,7 @@ class ChromeProxyService implements VmServiceInterface {
'The VM is unable to add a breakpoint '
'at the specified line or function');
}
final dartUri = DartUri(scriptUri, uri);
final dartUri = DartUri(scriptUri, root);
final ref = await _inspector.scriptRefFor(dartUri.serverPath);
return (await _debugger)
.addBreakpoint(isolateId, ref.id, line, column: column);
Expand Down
4 changes: 2 additions & 2 deletions dwds/lib/src/services/debug_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ class DebugService {
String hostname,
RemoteDebugger remoteDebugger,
ExecutionContext executionContext,
String tabUrl,
String root,
AssetReader assetReader,
LoadStrategy loadStrategy,
AppConnection appConnection,
Expand All @@ -225,7 +225,7 @@ class DebugService {
useSse ??= false;
final chromeProxyService = await ChromeProxyService.create(
remoteDebugger,
tabUrl,
root,
assetReader,
loadStrategy,
appConnection,
Expand Down
33 changes: 13 additions & 20 deletions dwds/lib/src/utilities/dart_uri.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'package:logging/logging.dart';
import 'package:package_config/package_config.dart';
import 'package:path/path.dart' as p;

import '../loaders/require.dart';
import '../loaders/strategy.dart';
import 'sdk_configuration.dart';

Expand All @@ -30,26 +29,22 @@ class DartUri {
/// path is a web server path and so relative to the directory being
/// served, not to the package.
///
/// The optional [serverUri] is a temporary workaround for a bug with construction.
/// Older SDKs (before D24) gave us a path that didn't include the full path,
/// e.g. main.dart rather than hello_world/main.dart and src/path.dart rather than
/// packages/path/src/path.dart. The optional [serverUri] is the full URI of the
/// JS script. The dirname of that path should give us the missing prefix.
factory DartUri(String uri, [String serverUri]) {
/// The optional [root] is the directory the app is served from.
factory DartUri(String uri, [String root]) {
final serverPath = globalLoadStrategy.serverPathForAppUri(uri);
if (serverPath != null) {
return DartUri._(serverPath);
}
// TODO(annagrin): Support creating DartUris from `dart:` uris.
// Issue: https://github.com/dart-lang/webdev/issues/1584
if (uri.startsWith('package:')) {
return DartUri._fromPackageUri(uri, serverUri: serverUri);
return DartUri._fromPackageUri(uri, root: root);
}
if (uri.startsWith('file:')) {
return DartUri._fromFileUri(uri, serverUri: serverUri);
return DartUri._fromFileUri(uri, root: root);
}
if (uri.startsWith('/packages/')) {
return DartUri._fromRelativePath(uri, serverUri: serverUri);
return DartUri._fromRelativePath(uri, root: root);
}
if (uri.startsWith('/')) {
return DartUri._fromRelativePath(uri);
Expand All @@ -65,32 +60,30 @@ class DartUri {
String toString() => 'DartUri: $serverPath';

/// Construct from a package: URI
factory DartUri._fromPackageUri(String uri, {String serverUri}) {
final basePath = basePathForServerUri(serverUri);
factory DartUri._fromPackageUri(String uri, {String root}) {
final packagePath = 'packages/${uri.substring("package:".length)}';
if (serverUri != null) {
final relativePath = p.url.join(basePath, packagePath);
if (root != null) {
final relativePath = p.url.join(root, packagePath);
return DartUri._fromRelativePath(relativePath);
}
return DartUri._(packagePath);
}

/// Construct from a file: URI
factory DartUri._fromFileUri(String uri, {String serverUri}) {
factory DartUri._fromFileUri(String uri, {String root}) {
final libraryName = _resolvedUriToUri[uri];
if (libraryName != null) return DartUri(libraryName, serverUri);
if (libraryName != null) return DartUri(libraryName, root);
// This is not one of our recorded libraries.
throw ArgumentError.value(uri, 'uri', 'Unknown library');
}

/// Construct from a path, relative to the directory being served.
factory DartUri._fromRelativePath(String uri, {String serverUri}) {
factory DartUri._fromRelativePath(String uri, {String root}) {
uri = uri[0] == '.' ? uri.substring(1) : uri;
uri = uri[0] == '/' ? uri.substring(1) : uri;

if (serverUri != null) {
final basePath = basePathForServerUri(serverUri);
return DartUri._fromRelativePath(p.url.join(basePath, uri));
if (root != null) {
return DartUri._fromRelativePath(p.url.join(root, uri));
}
return DartUri._(uri);
}
Expand Down
11 changes: 8 additions & 3 deletions dwds/test/build_daemon_callstack_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,20 @@ void main() {
// Enable verbose logging for debugging.
final debug = false;

for (var soundNullSafety in [false, true]) {
for (var nullSafety in NullSafety.values) {
final soundNullSafety = nullSafety == NullSafety.sound;
final setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound();
final context = setup.context;

group('${soundNullSafety ? "sound" : "weak"} null safety |', () {
group('${nullSafety.name} null safety |', () {
setUpAll(() async {
setCurrentLogWriter(debug: debug);
await context.setUp(
enableExpressionEvaluation: true, verboseCompiler: debug);
compilationMode: CompilationMode.buildDaemon,
nullSafety: nullSafety,
enableExpressionEvaluation: true,
verboseCompiler: debug,
);
});

tearDownAll(() async {
Expand Down
19 changes: 7 additions & 12 deletions dwds/test/build_daemon_evaluate_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,13 @@ void main() async {
// Enable verbose logging for debugging.
final debug = false;

for (var soundNullSafety in [false, true]) {
group('${soundNullSafety ? "sound" : "weak"} null safety |', () {
for (var basePath in ['', 'abc']) {
group('with base "$basePath" |', () {
testAll(
compilationMode: CompilationMode.buildDaemon,
soundNullSafety: soundNullSafety,
basePath: basePath,
debug: debug,
);
});
}
for (var nullSafety in NullSafety.values) {
group('${nullSafety.name} null safety |', () {
testAll(
compilationMode: CompilationMode.buildDaemon,
nullSafety: nullSafety,
debug: debug,
);
});
}
}
3 changes: 1 addition & 2 deletions dwds/test/dart_uri_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ void main() {
});

test('parses package : paths with root', () {
final uri = DartUri(
'package:path/path.dart', 'http://localhost:8080/foo/bar/blah');
final uri = DartUri('package:path/path.dart', 'foo/bar/blah');
expect(uri.serverPath, 'foo/bar/blah/packages/path/path.dart');
});

Expand Down
Loading