Support picking a port, use it in some tests. (#4212)

Author: davidmorgan

build_runner/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 2.8.1-wip
 
+- Print the port that gets picked if you pass 0 for a port number, for example
+  with `dart run build_runner serve web:0`.
 - Improved warnings when an option is specified for an unknown builder.
 - Bug fix: require `args` 2.5.0.
 - Use `build` ^4.0.0.

build_runner/lib/src/commands/serve_command.dart

@@ -58,9 +58,8 @@ class ServeCommand implements BuildRunnerCommand {
       } on SocketException catch (e) {
         if (e.address != null && e.port != null) {
           buildLog.error(
-            'Error starting server at ${e.address!.address}:${e.port}, address '
-            'is already in use. Please kill the server running on that port or '
-            'serve on a different port and restart this process.',
+            'Failed to start server on ${e.address!.address}:${e.port}, '
+            'address in use.',
           );
         } else {
           buildLog.error('Error starting server on ${serveOptions.hostname}.');
@@ -92,7 +91,18 @@ class ServeCommand implements BuildRunnerCommand {
 
       final completer = Completer<int>();
       handleBuildResultsStream(handler.buildResults, completer);
-      _logServerPorts();
+
+      if (serveOptions.serveTargets.isEmpty) {
+        buildLog.warning('Nothing to serve.');
+      } else {
+        for (final target in serveOptions.serveTargets) {
+          final port = servers[target]!.port;
+          buildLog.info(
+            'Serving `${target.dir}` on http://${serveOptions.hostname}:$port',
+          );
+        }
+      }
+
       await handler.currentBuild;
       return await completer.future;
     } finally {
@@ -101,36 +111,18 @@ class ServeCommand implements BuildRunnerCommand {
       );
     }
   }
-
-  void _logServerPorts() async {
-    // Warn if in serve mode with no servers.
-    if (serveOptions.serveTargets.isEmpty) {
-      buildLog.warning(
-        'Found no known web directories to serve, but running in `serve` '
-        'mode. You may expliclity provide a directory to serve with trailing '
-        'args in <dir>[:<port>] format.',
-      );
-    } else {
-      for (final target in serveOptions.serveTargets) {
-        buildLog.info(
-          'Serving `${target.dir}` on '
-          'http://${serveOptions.hostname}:${target.port}',
-        );
-      }
-    }
-  }
 }
 
 void _ensureBuildWebCompilersDependency(PackageGraph packageGraph) {
   if (!packageGraph.allPackages.containsKey('build_web_compilers')) {
     buildLog.warning('''
-    Missing dev dependency on package:build_web_compilers, which is required to serve Dart compiled to JavaScript.
+Missing dev dependency on package:build_web_compilers, which is required to serve Dart compiled to JavaScript.
 
-    Please update your dev_dependencies section of your pubspec.yaml:
+Please update your dev_dependencies section of your pubspec.yaml:
 
-    dev_dependencies:
-      build_runner: any
-      build_test: any
-      build_web_compilers: any''');
+dev_dependencies:
+  build_runner: any
+  build_test: any
+  build_web_compilers: any''');
   }
 }

build_runner/lib/src/commands/test_command.dart

@@ -60,11 +60,11 @@ Missing dev dependency on package:build_test, which is required to run tests.
 
 Please update your dev_dependencies section of your pubspec.yaml:
 
-  dev_dependencies:
-    build_runner: any
-    build_test: any
-    # If you need to run web tests, you will also need this dependency.
-    build_web_compilers: any''');
+dev_dependencies:
+  build_runner: any
+  build_test: any
+  # If you need to run web tests, you will also need this dependency.
+  build_web_compilers: any''');
         return ExitCode.config.code;
       }
 

build_runner/test/common/build_runner_tester.dart

@@ -175,8 +175,9 @@ ${result.stdout}${result.stderr}===
 
 /// A running `build_runner` process.
 class BuildRunnerProcess {
-  late final HttpClient _client = HttpClient();
   final TestProcess process;
+  late final HttpClient _client = HttpClient();
+  int? _port;
 
   BuildRunnerProcess(this.process);
 
@@ -188,8 +189,8 @@ class BuildRunnerProcess {
   ///
   /// If the process exits instead, the test fails immediately.
   ///
-  /// Otherwise, waits until [pattern] appears.
-  Future<void> expect(Pattern pattern, {Pattern? failOn}) async {
+  /// Otherwise, waits until [pattern] appears, returns the matching line.
+  Future<String> expect(Pattern pattern, {Pattern? failOn}) async {
     failOn ??= BuildLog.failurePattern;
     while (true) {
       String? line;
@@ -202,26 +203,36 @@ class BuildRunnerProcess {
       if (line.contains(failOn)) {
         fail('While expecting `$pattern`, got `$failOn`.');
       }
-      if (line.contains(pattern)) return;
+      if (line.contains(pattern)) return line;
     }
   }
 
   /// Kills the process.
   Future<void> kill() => process.kill();
 
-  /// Requests [path] from the default server and expects it returns a 404
+  // Expects the server to log that it is serving, records the port.
+  Future<void> expectServing() async {
+    final regexp = RegExp('Serving `web` on http://localhost:([0-9]+)');
+    final line = await expect(regexp);
+    final port = int.parse(regexp.firstMatch(line)!.group(1)!);
+    _port = port;
+  }
+
+  /// Requests [path] from the server and expects it returns a 404
   /// response.
   Future<void> expect404(String path) async {
-    final request = await _client.get('localhost', 8080, path);
+    if (_port == null) throw StateError('Call expectServing first.');
+    final request = await _client.get('localhost', _port!, path);
     final response = await request.close();
     test.expect(response.statusCode, 404);
     await response.drain<void>();
   }
 
-  /// Requests [path] from the default server and expects it returns a 200
+  /// Requests [path] from the server and expects it returns a 200
   /// response with the body [content].
   Future<void> expectContent(String path, String content) async {
-    final request = await _client.get('localhost', 8080, path);
+    if (_port == null) throw StateError('Call expectServing first.');
+    final request = await _client.get('localhost', _port!, path);
     final response = await request.close();
     test.expect(response.statusCode, 200);
     test.expect(await utf8.decodeStream(response.cast<List<int>>()), content);

build_runner/test/integration_tests/serve_command_serve_only_required_test.dart

@@ -2,8 +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.
 
-// TODO(davidmorgan): find unused port so this can run in parallel.
-@Tags(['slow'])
+@Tags(['integration'])
 library;
 
 import 'package:build_runner/src/logging/build_log.dart';
@@ -24,9 +23,13 @@ void main() async {
       files: {'web/a.txt': 'a', 'web/b.txt': 'b'},
     );
 
-    final serve = await tester.start('root_pkg', 'dart run build_runner serve');
+    final serve = await tester.start(
+      'root_pkg',
+      'dart run build_runner serve web:0',
+    );
 
     // Initial build produces no output as the copy is not required.
+    await serve.expectServing();
     await serve.expect(BuildLog.successPattern);
     await serve.expect404('a.txt.copy');
 

build_runner/test/integration_tests/serve_command_test.dart

@@ -2,7 +2,6 @@
 // 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.
 
-// TODO(davidmorgan): find unused port so this can run in parallel.
 @Tags(['integration'])
 library;
 
@@ -29,26 +28,26 @@ void main() async {
       'Missing dev dependency on package:build_web_compilers, '
       'which is required to serve Dart compiled to JavaScript.',
     );
-    await serve.expect('Found no known web directories to serve');
+    await serve.expect('Nothing to serve.');
     await serve.kill();
 
     // Create some source to serve.
     tester.write('root_pkg/web/a.txt', 'a');
 
-    // Start a server on the same port, serve, check the error.
-    final server = await HttpServer.bind('localhost', 8080);
+    // Start a server serve on the same port, check the error.
+    final server = await HttpServer.bind('localhost', 0);
     addTearDown(server.close);
     final output = await tester.run(
       'root_pkg',
-      'dart run build_runner serve web:8080',
+      'dart run build_runner serve web:${server.port}',
       expectExitCode: ExitCode.osError.code,
     );
     expect(
       output,
       allOf(
-        contains('Error starting server'),
-        contains('8080'),
-        contains('address is already in use'),
+        contains('Failed to start server'),
+        contains('${server.port}'),
+        contains('address in use'),
       ),
     );
     await server.close();