Run frontend server tests and fix failures (#1658)

* Run frontend server tests and fix failures

* Fix typo

* Fix analyzer warnings

* Pass uri instead of strings where required

* Fixed some failing tests on windows and disabled others

Author: Anna Gringauze

dwds/CHANGELOG.md

@@ -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.

dwds/lib/src/debugging/location.dart

@@ -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';
@@ -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);

dwds/lib/src/handlers/dev_handler.dart

@@ -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';
@@ -201,7 +202,7 @@ class DevHandler {
       'localhost',
       webkitDebugger,
       executionContext,
-      appTab.url,
+      basePathForServerUri(appTab.url),
       _assetReader,
       _loadStrategy,
       appConnection,

dwds/lib/src/loaders/frontend_server_require.dart

@@ -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,
@@ -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

dwds/lib/src/loaders/require.dart

@@ -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) =>

dwds/lib/src/services/chrome_proxy_service.dart

@@ -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;
@@ -104,7 +104,7 @@ class ChromeProxyService implements VmServiceInterface {
 
   ChromeProxyService._(
     this._vm,
-    this.uri,
+    this.root,
     this._assetReader,
     this.remoteDebugger,
     this._modules,
@@ -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,
@@ -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,
@@ -229,7 +229,7 @@ class ChromeProxyService implements VmServiceInterface {
       remoteDebugger,
       _assetReader,
       _locations,
-      uri,
+      root,
       debugger,
       executionContext,
       sdkConfiguration,
@@ -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);

dwds/lib/src/services/debug_service.dart

@@ -210,7 +210,7 @@ class DebugService {
     String hostname,
     RemoteDebugger remoteDebugger,
     ExecutionContext executionContext,
-    String tabUrl,
+    String root,
     AssetReader assetReader,
     LoadStrategy loadStrategy,
     AppConnection appConnection,
@@ -225,7 +225,7 @@ class DebugService {
     useSse ??= false;
     final chromeProxyService = await ChromeProxyService.create(
       remoteDebugger,
-      tabUrl,
+      root,
       assetReader,
       loadStrategy,
       appConnection,

dwds/lib/src/utilities/dart_uri.dart

@@ -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';
 
@@ -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);
@@ -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);
   }

dwds/test/build_daemon_callstack_test.dart

@@ -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 {

dwds/test/build_daemon_evaluate_test.dart

@@ -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,
+      );
     });
   }
 }