delete packageRoot support in test runner package, migrate to package_config from package_resolver

Not sure what might be relying on this - going to check what the bots think about it :D

Bug: https://github.com/dart-lang/package_resolver/issues/30
Change-Id: I283d60d749a3db8a4e02dfdb1889ce56c8630620
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138324
Commit-Queue: Jake Macdonald <jakemac@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>

pkg/test_runner/bin/http_server.dart

@@ -20,7 +20,6 @@ void main(List<String> arguments) {
   parser.addFlag('help',
       abbr: 'h', negatable: false, help: 'Print this usage information.');
   parser.addOption('build-directory', help: 'The build directory to use.');
-  parser.addOption('package-root', help: 'The package root to use.');
   parser.addOption('packages', help: 'The package spec file to use.');
   parser.addOption('network',
       help: 'The network interface to use.', defaultsTo: '0.0.0.0');
@@ -38,7 +37,6 @@ void main(List<String> arguments) {
         args['csp'] as bool,
         Runtime.find(args['runtime'] as String),
         null,
-        args['package-root'] as String,
         args['packages'] as String);
     var port = int.parse(args['port'] as String);
     var crossOriginPort = int.parse(args['crossOriginPort'] as String);

pkg/test_runner/lib/src/configuration.dart

@@ -63,7 +63,6 @@ class TestConfiguration {
       this.keepGeneratedFiles,
       this.sharedOptions,
       String packages,
-      this.packageRoot,
       this.suiteDirectory,
       this.outputDirectory,
       this.reproducingArguments,
@@ -170,15 +169,12 @@ class TestConfiguration {
 
   String get packages {
     // If the .packages file path wasn't given, find it.
-    if (packageRoot == null && _packages == null) {
-      _packages = Repository.uri.resolve('.packages').toFilePath();
-    }
+    _packages ??= Repository.uri.resolve('.packages').toFilePath();
 
     return _packages;
   }
 
   final String outputDirectory;
-  final String packageRoot;
   final String suiteDirectory;
   String get babel => configuration.babel;
   String get builderTag => configuration.builderTag;
@@ -431,8 +427,7 @@ class TestConfiguration {
   /// server for cross-domain tests can be found by calling
   /// `getCrossOriginPortNumber()`.
   Future startServers() {
-    _servers = TestingServers(
-        buildDirectory, isCsp, runtime, null, packageRoot, packages);
+    _servers = TestingServers(buildDirectory, isCsp, runtime, null, packages);
     var future = servers.startServers(localIP,
         port: testServerPort, crossOriginPort: testServerCrossOriginPort);
 

pkg/test_runner/lib/src/options.dart

@@ -747,7 +747,6 @@ compiler.''',
           localIP: data["local_ip"] as String,
           sharedOptions: sharedOptions,
           packages: data["packages"] as String,
-          packageRoot: data["package_root"] as String,
           suiteDirectory: data["suite_dir"] as String,
           outputDirectory: data["output_directory"] as String,
           reproducingArguments:

pkg/test_runner/lib/src/test_file.dart

@@ -18,7 +18,6 @@ final _multitestRegExp = RegExp(r"\S *//[#/] \w+:(.*)");
 
 final _vmOptionsRegExp = RegExp(r"// VMOptions=(.*)");
 final _environmentRegExp = RegExp(r"// Environment=(.*)");
-final _packageRootRegExp = RegExp(r"// PackageRoot=(.*)");
 final _packagesRegExp = RegExp(r"// Packages=(.*)");
 final _experimentRegExp = RegExp(r"^--enable-experiment=([a-z,-]+)$");
 
@@ -180,7 +179,6 @@ class TestFile extends _TestFileBase {
           dart2jsOptions: [],
           ddcOptions: [],
           dartOptions: [],
-          packageRoot: null,
           packages: null,
           hasSyntaxError: false,
           hasCompileError: false,
@@ -259,34 +257,17 @@ class TestFile extends _TestFileBase {
     }
 
     // Packages.
-    String packageRoot;
     String packages;
-    matches = _packageRootRegExp.allMatches(contents);
-    for (var match in matches) {
-      if (packageRoot != null || packages != null) {
-        throw Exception('More than one "// Package... line in test $filePath');
-      }
-      packageRoot = match[1];
-      if (packageRoot != 'none') {
-        // PackageRoot=none means that no packages or package-root option
-        // should be given. Any other value overrides package-root and
-        // removes any packages option.  Don't use with // Packages=.
-        packageRoot = Uri.file(filePath)
-            .resolveUri(Uri.directory(packageRoot))
-            .toFilePath();
-      }
-    }
 
     matches = _packagesRegExp.allMatches(contents);
     for (var match in matches) {
-      if (packages != null || packageRoot != null) {
+      if (packages != null) {
         throw Exception('More than one "// Package..." line in test $filePath');
       }
       packages = match[1];
       if (packages != 'none') {
-        // Packages=none means that no packages or package-root option
-        // should be given. Any other value overrides packages and removes
-        // any package-root option. Don't use with // PackageRoot=.
+        // Packages=none means that no packages option should be given. Any
+        // other value overrides packages.
         packages =
             Uri.file(filePath).resolveUri(Uri.file(packages)).toFilePath();
       }
@@ -338,7 +319,6 @@ class TestFile extends _TestFileBase {
     }
 
     return TestFile._(suiteDirectory, Path(filePath), errorExpectations,
-        packageRoot: packageRoot,
         packages: packages,
         environment: environment,
         isMultitest: isMultitest,
@@ -367,8 +347,7 @@ class TestFile extends _TestFileBase {
       this.hasRuntimeError,
       this.hasStaticWarning,
       this.hasCrash})
-      : packageRoot = null,
-        packages = null,
+      : packages = null,
         environment = null,
         isMultitest = false,
         isMultiHtmlTest = false,
@@ -385,8 +364,7 @@ class TestFile extends _TestFileBase {
         super(null, null, []);
 
   TestFile._(Path suiteDirectory, Path path, List<StaticError> expectedErrors,
-      {this.packageRoot,
-      this.packages,
+      {this.packages,
       this.environment,
       this.isMultitest,
       this.isMultiHtmlTest,
@@ -413,7 +391,6 @@ class TestFile extends _TestFileBase {
 
   String get multitestKey => "";
 
-  final String packageRoot;
   final String packages;
 
   final Map<String, String> environment;
@@ -465,7 +442,6 @@ class TestFile extends _TestFileBase {
           hasSyntaxError: hasSyntaxError ?? false);
 
   String toString() => """TestFile(
-  packageRoot: $packageRoot
   packages: $packages
   environment: $environment
   isMultitest: $isMultitest
@@ -514,7 +490,6 @@ class _MultitestFile extends _TestFileBase implements TestFile {
 
   Path get originPath => _origin.path;
 
-  String get packageRoot => _origin.packageRoot;
   String get packages => _origin.packages;
 
   List<Feature> get requirements => _origin.requirements;

pkg/test_runner/lib/src/test_suite.dart

@@ -782,8 +782,7 @@ class StandardTestSuite extends TestSuite {
             _createUrlPathFromFile(Path('$compilationTempDir/$nameNoExt.js'));
         content = dart2jsHtml(testFile.path.toNativePath(), scriptPath);
       } else {
-        var packageRoot =
-            packagesArgument(configuration.packageRoot, configuration.packages);
+        var packageRoot = packagesArgument(configuration.packages);
         packageRoot =
             packageRoot == null ? nameNoExt : packageRoot.split("=").last;
         var nameFromModuleRoot =
@@ -860,7 +859,7 @@ class StandardTestSuite extends TestSuite {
   List<String> _commonArgumentsFromFile(TestFile testFile) {
     var args = configuration.standardOptions.toList();
 
-    var packages = packagesArgument(testFile.packageRoot, testFile.packages);
+    var packages = packagesArgument(testFile.packages);
     if (packages != null) {
       args.add(packages);
     }
@@ -880,25 +879,17 @@ class StandardTestSuite extends TestSuite {
     return args;
   }
 
-  String packagesArgument(String packageRoot, String packages) {
+  String packagesArgument(String packages) {
     // If this test is inside a package, we will check if there is a
     // pubspec.yaml file and if so, create a custom package root for it.
-    if (packageRoot == null && packages == null) {
-      if (configuration.packageRoot != null) {
-        packageRoot = Path(configuration.packageRoot).toNativePath();
-      }
-
-      if (configuration.packages != null) {
-        packages = Path(configuration.packages).toNativePath();
-      }
+    if (packages == null && configuration.packages != null) {
+      packages = Path(configuration.packages).toNativePath();
     }
 
-    if (packageRoot == 'none' || packages == 'none') {
+    if (packages == 'none') {
       return null;
     } else if (packages != null) {
       return '--packages=$packages';
-    } else if (packageRoot != null) {
-      return '--package-root=$packageRoot';
     } else {
       return null;
     }

pkg/test_runner/lib/src/testing_servers.dart

@@ -6,7 +6,7 @@ import 'dart:async';
 import 'dart:convert' show HtmlEscape;
 import 'dart:io';
 
-import 'package:package_resolver/package_resolver.dart';
+import 'package:package_config/package_config.dart';
 
 import 'package:test_runner/src/configuration.dart';
 import 'package:test_runner/src/repository.dart';
@@ -74,35 +74,28 @@ class TestingServers {
   ];
 
   final List<HttpServer> _serverList = [];
-  Uri _buildDirectory;
-  Uri _dartDirectory;
-  Uri _packageRoot;
-  Uri _packages;
+  final Uri _buildDirectory;
+  final Uri _dartDirectory;
+  final Uri _packages;
+  PackageConfig _packageConfig;
   final bool useContentSecurityPolicy;
   final Runtime runtime;
   DispatchingServer _server;
-  SyncPackageResolver _resolver;
 
-  TestingServers(String buildDirectory, this.useContentSecurityPolicy,
-      [this.runtime = Runtime.none,
-      String dartDirectory,
-      String packageRoot,
-      String packages]) {
-    _buildDirectory = Uri.base.resolveUri(Uri.directory(buildDirectory));
-    if (dartDirectory == null) {
-      _dartDirectory = Repository.uri;
-    } else {
-      _dartDirectory = Uri.base.resolveUri(Uri.directory(dartDirectory));
-    }
-    if (packageRoot == null) {
-      if (packages == null) {
-        _packages = _dartDirectory.resolve('.packages');
-      } else {
-        _packages = Uri.file(packages);
-      }
-    } else {
-      _packageRoot = Uri.directory(packageRoot);
-    }
+  TestingServers._(this.useContentSecurityPolicy, this._buildDirectory,
+      this._dartDirectory, this._packages, this.runtime);
+
+  factory TestingServers(String buildDirectory, bool useContentSecurityPolicy,
+      [Runtime runtime = Runtime.none, String dartDirectory, String packages]) {
+    var buildDirectoryUri = Uri.base.resolveUri(Uri.directory(buildDirectory));
+    var dartDirectoryUri = dartDirectory == null
+        ? Repository.uri
+        : Uri.base.resolveUri(Uri.directory(dartDirectory));
+    var packagesUri = packages == null
+        ? dartDirectoryUri.resolve('.packages')
+        : Uri.base.resolve(packages);
+    return TestingServers._(useContentSecurityPolicy, buildDirectoryUri,
+        dartDirectoryUri, packagesUri, runtime);
   }
 
   String get network => _serverList[0].address.address;
@@ -122,11 +115,8 @@ class TestingServers {
   ///   "Access-Control-Allow-Credentials: true"
   Future startServers(String host,
       {int port = 0, int crossOriginPort = 0}) async {
-    if (_packages != null) {
-      _resolver = await SyncPackageResolver.loadConfig(_packages);
-    } else {
-      _resolver = SyncPackageResolver.root(_packageRoot);
-    }
+    _packageConfig = await loadPackageConfigUri(_packages);
+
     _server = await _startHttpServer(host, port: port);
     await _startHttpServer(host,
         port: crossOriginPort, allowedPort: _serverList[0].port);
@@ -150,10 +140,7 @@ class TestingServers {
       '--build-directory=$buildDirectory',
       '--runtime=${runtime.name}',
       if (useContentSecurityPolicy) '--csp',
-      if (_packages != null)
-        '--packages=${_packages.toFilePath()}'
-      else if (_packageRoot != null)
-        '--package-root=${_packageRoot.toFilePath()}'
+      '--packages=${_packages.toFilePath()}'
     ].join(' ');
   }
 
@@ -260,7 +247,7 @@ class TestingServers {
       var packageUri = Uri(
           scheme: 'package',
           pathSegments: pathSegments.skip(packagesIndex + 1));
-      return _resolver.resolveUri(packageUri);
+      return _packageConfig.resolve(packageUri);
     }
     if (pathSegments[0] == prefixBuildDir) {
       return _buildDirectory.resolve(pathSegments.skip(1).join('/'));

pkg/test_runner/pubspec.yaml

@@ -11,8 +11,8 @@ dependencies:
     path: ../expect
   glob:
     path: ../../third_party/pkg/glob
-  package_resolver:
-    path: ../../third_party/pkg_tested/package_resolver
+  package_config:
+    path: ../../third_party/pkg_tested/package_config
   smith:
     path: ../smith
   status_file:

tests/lib_2/isolate/scenarios/bad_resolve_package/bad_resolve_package_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 import 'dart:io';
 import 'dart:isolate';

tests/lib_2/isolate/scenarios/package_relative_root/package_relative_root_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 import 'dart:io';
 import 'dart:isolate';

tests/lib_2/isolate/scenarios/package_relative_spec/package_relative_spec_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 import 'dart:io';
 import 'dart:isolate';

tests/lib_2/isolate/scenarios/short_package/short_package_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 import 'dart:io';
 import 'dart:isolate';

tests/standalone_2/package/package1_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library package1_test;
 

tests/standalone_2/package/package_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library package_test;
 

tests/standalone_2/package/scenarios/both_dir_and_file/both_dir_and_file_noimports_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library both_dir_and_file_noimports_test;
 

tests/standalone_2/package/scenarios/both_dir_and_file/prefers_packages_file_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library prefers_packages_file_test;
 

tests/standalone_2/package/scenarios/empty_packages_file/empty_packages_file_discovery_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library empty_packages_file_discovery_test;
 

tests/standalone_2/package/scenarios/packages_dir_only/packages/foo/foo.dart

@@ -1,7 +0,0 @@
-// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
-// 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.
-
-library foo;
-
-String bar = 'hello';

tests/standalone_2/package/scenarios/packages_dir_only/packages_dir_only_noimports_test.dart

@@ -1,9 +0,0 @@
-// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
-// 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.
-
-// PackageRoot=none
-
-library packages_dir_only_noimports_test;
-
-main() {}

tests/standalone_2/package/scenarios/packages_dir_only/packages_dir_only_test.dart

@@ -1,15 +0,0 @@
-// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
-// 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.
-
-// PackageRoot=none
-
-library packages_dir_only_test;
-
-import 'package:foo/foo.dart' as foo;
-
-main() {
-  if (foo.bar != 'hello') {
-    throw new Exception('package "foo" was not resolved correctly');
-  }
-}

tests/standalone_2/package/scenarios/packages_file_in_parent/sub/packages_file_in_parent_noimports_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library packages_file_in_parent_noimports_test;
 

tests/standalone_2/package/scenarios/packages_file_in_parent/sub/packages_file_in_parent_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library packages_file_in_parent_test;
 

tests/standalone_2/package/scenarios/packages_file_only/packages_file_only_noimports_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library packages_file_only_noimports_test;
 

tests/standalone_2/package/scenarios/packages_file_only/packages_file_only_test.dart

@@ -2,7 +2,7 @@
 // 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.
 
-// PackageRoot=none
+// Packages=none
 
 library packages_file_only_test;