Start two http servers for every configuration

tools/testing/dart/http_server.dart

Index: tools/testing/dart/http_server.dart
diff --git a/tools/testing/dart/http_server.dart b/tools/testing/dart/http_server.dart
index 6ab495b7af70baa95076b66026ba5a2b75656377..27466a56a476cba0e1c0ef41420ec686401260b5 100644
--- a/tools/testing/dart/http_server.dart
+++ b/tools/testing/dart/http_server.dart
@@ -37,7 +37,6 @@ const PREFIX_DARTDIR = 'root_dart';
 // http://host:port/root_packages/X -> $BuildDir/packages/X
 // Issue: 8368
 
-
 main() {
   /** Convenience method for local testing. */
   var parser = new ArgParser();
@@ -47,13 +46,9 @@ main() {
   parser.addOption('crossOriginPort', abbr: 'c',
       help: 'A different port that accepts request from the main server port.',
       defaultsTo: '0');
-  parser.addOption('mode', abbr: 'm', help: 'Testing mode.',
-      defaultsTo: 'release');
-  parser.addOption('arch', abbr: 'a', help: 'Testing architecture.',
-      defaultsTo: 'ia32');
   parser.addFlag('help', abbr: 'h', negatable: false,
       help: 'Print this usage information.');
-  parser.addOption('package-root', help: 'The package root to use.');
+  parser.addOption('build-directory', help: 'The package root to use.');
   parser.addOption('network', help: 'The network interface to use.',
       defaultsTo: '127.0.0.1');
   var args = parser.parse(new Options().arguments);
@@ -68,81 +63,96 @@ main() {
         .join(new Path('../../test.dart'))
         .canonicalize()
         .toNativePath();
-    // Note: args['package-root'] is always the build directory. We have the
-    // implicit assumption that it contains the 'packages' subdirectory.
-    // TODO: We should probably rename 'package-root' to 'build-directory'.
-    TestingServerRunner._packageRootDir = new Path(args['package-root']);
-    TestingServerRunner._buildDirectory = new Path(args['package-root']);
-    var network = args['network'];
-    TestingServerRunner.startHttpServer(network,
-        port: int.parse(args['port']));
-    print('Server listening on port '
-          '${TestingServerRunner.serverList[0].port}.');
-    TestingServerRunner.startHttpServer(network,
-        allowedPort: TestingServerRunner.serverList[0].port, port:
-        int.parse(args['crossOriginPort']));
-    print(
-        'Server listening on port ${TestingServerRunner.serverList[1].port}.');
+    var servers = new TestingServers(new Path(args['build-directory']));
+    var port = int.parse(args['port']);
+    var crossOriginPort = int.parse(args['crossOriginPort']);
+    servers.startServers(args['network'],
+                         port: port,
+                         crossOriginPort: crossOriginPort);
+    DebugLogger.info('Server listening on port ${servers.port}');
+    DebugLogger.info('Server listening on port ${servers.crossOriginPort}');
   }
 }
+
 /**
  * Runs a set of servers that are initialized specifically for the needs of our
  * test framework, such as dealing with package-root.
  */
-class TestingServerRunner {
-  static List serverList = [];
-  static Path _packageRootDir = null;
-  static Path _buildDirectory = null;
+class TestingServers {
+  List _serverList = [];
+  Path _buildDirectory = null;
+
+  TestingServers(Path buildDirectory) {
+    _buildDirectory = TestUtils.absolutePath(buildDirectory);
+  }
 
-  // Added as a getter so that the function will be called again each time the
-  // default request handler closure is executed.
-  static Path get packageRootDir => _packageRootDir;
-  static Path get buildDirectory => _buildDirectory;
+  int get port => _serverList[0].port;
+  int get crossOriginPort => _serverList[1].port;
 
-  static setPackageRootDir(Map configuration) {
-    _packageRootDir = TestUtils.absolutePath(
-        new Path(TestUtils.buildDir(configuration)));
+  /**
+   * [startHttpServer] will start two Http servers.
+   * The first server listens on [port] and sets
+   *   "Access-Control-Allow-Origin: *"
+   * The second server listens on [crossOriginPort] and sets
+   *   "Access-Control-Allow-Origin: client:port1
+   *   "Access-Control-Allow-Credentials: true"
+   */
+  void startServers(String host, {int port: 0, int crossOriginPort: 0}) {
+    _startHttpServer(host, port: port);
+    _startHttpServer(host,
+                     port: crossOriginPort,
+                     allowedPort:_serverList[0].port);
   }
 
-  static setBuildDir(Map configuration) {
-    _buildDirectory = TestUtils.absolutePath(
-        new Path(TestUtils.buildDir(configuration)));
+  String httpServerCommandline() {

[Emily Fortuna, 2013/02/22 17:45:30]

:-)

+    var dart = TestUtils.dartTestExecutable.toNativePath();
+    var dartDir = TestUtils.dartDir();
+    var script = dartDir.join(new Path("tools/testing/dart/http_server.dart"));
+    var buildDirectory = _buildDirectory.toNativePath();
+
+    return '$dart $script -p $port -c $crossOriginPort '
+           '--build-directory=$buildDirectory';
   }
 
-  static startHttpServer(String host, {int allowedPort:-1, int port: 0}) {
+  void stopServers() {
+    for (var server in _serverList) {
+      server.close();
+    }
+  }
+
+  void _startHttpServer(String host, {int port: 0, int allowedPort: -1}) {
     var httpServer = new HttpServer();
     httpServer.onError = (e) {
       DebugLogger.error('HttpServer: an error occured: $e');
     };
     httpServer.defaultRequestHandler = (request, response) {
-      handleFileOrDirectoryRequest(request, response, allowedPort);
+      _handleFileOrDirectoryRequest(request, response, allowedPort);
     };
     httpServer.addRequestHandler(
-        (req) => req.path == "/echo", handleEchoRequest);
+        (req) => req.path == "/echo", _handleEchoRequest);
 
     httpServer.listen(host, port);
-    serverList.add(httpServer);
+    _serverList.add(httpServer);
   }
 
-
-  static void handleFileOrDirectoryRequest(HttpRequest request,
-                                           HttpResponse response,
-                                           int allowedPort) {
-    var path = getFilePathFromRequestPath(request.path);
+  void _handleFileOrDirectoryRequest(HttpRequest request,
+                                     HttpResponse response,
+                                     int allowedPort) {
+    var path = _getFilePathFromRequestPath(request.path);
     if (path != null) {
       var file = new File.fromPath(path);
       file.exists().then((exists) {
         if (exists) {
-          sendFileContent(request, response, allowedPort, path, file);
+          _sendFileContent(request, response, allowedPort, path, file);
         } else {
           var directory = new Directory.fromPath(path);
           directory.exists().then((exists) {
             if (exists) {
-              listDirectory(directory).then((entries) {
-                sendDirectoryListing(entries, request, response);
+              _listDirectory(directory).then((entries) {
+                _sendDirectoryListing(entries, request, response);
               });
             } else {
-              sendNotFound(request, response);
+              _sendNotFound(request, response);
             }
           });
         }
@@ -152,19 +162,19 @@ class TestingServerRunner {
         var entries = [new _Entry('root_dart', 'root_dart/'),
                        new _Entry('root_build', 'root_build/'),
                        new _Entry('echo', 'echo')];
-        sendDirectoryListing(entries, request, response);
+        _sendDirectoryListing(entries, request, response);
       } else {
-        sendNotFound(request, response);
+        _sendNotFound(request, response);
       }
     }
   }
 
-  static void handleEchoRequest(HttpRequest request, HttpResponse response) {
+  void _handleEchoRequest(HttpRequest request, HttpResponse response) {
     response.headers.set("Access-Control-Allow-Origin", "*");
     request.inputStream.pipe(response.outputStream);
   }
 
-  static Path getFilePathFromRequestPath(String urlRequestPath) {
+  Path _getFilePathFromRequestPath(String urlRequestPath) {
     // Go to the top of the file to see an explanation of the URL path scheme.
     var requestPath = new Path(urlRequestPath.substring(1)).canonicalize();
     var pathSegments = requestPath.segments();
@@ -185,7 +195,7 @@ class TestingServerRunner {
       if (packagesIndex != -1) {
         var start = packagesIndex + 1;
         var length = pathSegments.length - start;
-        basePath = _packageRootDir.append(packagesDirName);
+        basePath = _buildDirectory.append(packagesDirName);
         relativePath = new Path(
             pathSegments.getRange(start, length).join('/'));
       }
@@ -196,7 +206,7 @@ class TestingServerRunner {
     return null;
   }
 
-  static Future<List<_Entry>> listDirectory(Directory directory) {
+  Future<List<_Entry>> _listDirectory(Directory directory) {
     var completer = new Completer();
     var entries = [];
 
@@ -215,16 +225,9 @@ class TestingServerRunner {
     return completer.future;
   }
 
-  /**
-   * Sends a simple listing of all the files and sub-directories within
-   * directory.
-   *
-   * This is intended to make it easier to browse tests when manually running

[Emily Fortuna, 2013/02/22 17:45:30]

why delete the documentation here? The functionality didn't seem to have
changed...

[kustermann, 2013/02/22 18:16:38]

Well it did change a little bit (but not in this CL, but rather in the 'putting
generated_tests into the build directory'-CL).

The first part of the comment refers to '... within *directory*'. We no longer
pass in the directory as an argument but rather the entries it should list.
The second part of the comment doesn't really describe what the function does
(but rather why we call this function).

I don't have strong opinions about keeping/deleting the comment. But IMHO it's
not necessary (the second part could be moved to the call-sites of
sendDirectoryListing).

[Emily Fortuna, 2013/02/22 18:42:51]

Err on the side of keeping the documentation that explains why the function is
useful. It's intended for people that are unaware we have this functionality
(same goes for the testing server in general) to make it more discoverable for
them should they look at the http_server file.

[kustermann, 2013/02/25 11:51:22]

IMHO, the people that are unaware of what the http server does should only need
to read the documentation at the top of the file (line 30).

-   * tests against this test server.
-   */
-  static void sendDirectoryListing(entries,
-                                   HttpRequest request,
-                                   HttpResponse response) {
+  void _sendDirectoryListing(List<_Entry> entries,
+                             HttpRequest request,
+                             HttpResponse response) {
     response.headers.set('Content-Type', 'text/html');
     var header = '''<!DOCTYPE html>
     <html>
@@ -254,11 +257,11 @@ class TestingServerRunner {
     response.outputStream.close();
   }
 
-  static void sendFileContent(HttpRequest request,
-                              HttpResponse response,
-                              int allowedPort,
-                              Path path,
-                              File file) {
+  void _sendFileContent(HttpRequest request,
+                        HttpResponse response,
+                        int allowedPort,
+                        Path path,
+                        File file) {
     if (allowedPort != -1) {
       var origin = new Uri(request.headers.value('Origin'));
       // Allow loading from http://*:$allowedPort in browsers.
@@ -281,7 +284,7 @@ class TestingServerRunner {
     file.openInputStream().pipe(response.outputStream);
   }
 
-  static void sendNotFound(HttpRequest request, HttpResponse response) {
+  void _sendNotFound(HttpRequest request, HttpResponse response) {
     // NOTE: Since some tests deliberately try to access non-existent files.
     // We might want to remove this warning (otherwise it will show
     // up in the debug.log every time).
@@ -299,10 +302,6 @@ class TestingServerRunner {
       }
     }
   }
-
-  static terminateHttpServers() {
-    for (var server in serverList) server.close();
-  }
 }
 
 // Helper class for displaying directory listings.