Make browser tests all run from a server instead of the local filesystem.

tools/testing/dart/http_server.dart

 // Copyright (c) 2012, 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 http_server;
 
 import 'dart:io';
 import 'dart:isolate';
 import 'test_suite.dart';  // For TestUtils.
+import '../../../pkg/args/lib/args.dart';
+import '../../../pkg/path/lib/path.dart' as pathLib;
 
-HttpServer _httpServer;
+main() {

[Emily Fortuna, 2013/01/10 02:07:10]

Q: What happened to this file? 
A: The main issue that caused all these changes was the dartium checkout output
directory is in a different location than the regular dart checkout build
directory. This causes problems when trying to set the package root in a way
that works on both checkouts, without passing on this complication to the user
writing tests. 

With the addition of the DART_PACKAGE_ROOT environment variable (added to drt in
a previous CL), we could no longer simply run the commands printed out by the
buildbot (or from our command line) to reproduce the environment of the test,
since we had to know what the package root value is, too. For debugabilty, I
added a command in the testing printout that lists the command to start this
file up as a local server for debugging.

[ahe, 2013/01/10 09:27:24]

Great idea!

+  /** Convenience method for local testing. */
+  var parser = new ArgParser();
+  parser.addOption('port', abbr: 'p',
+      help: 'The main server port we wish to respond to requests.',
+      defaultsTo: '0');
+  parser.addOption('crossOriginPort', abbr: 'c',
+      help: 'A different port that accepts request from the main server port.',
+      defaultsTo: '0');
+  parser.addFlag('help', abbr: 'h', negatable: false,
+      help: 'Print this usage information.');
+  var args = parser.parse(new Options().arguments);
+  if (args['help']) {
+    print(parser.getUsage());
+  } else {
+    // Pretend we're running test.dart so that TestUtils doesn't get confused
+    // about the "current directory." This is only used if we're trying to run
+    // this file independently for local testing.
+    TestUtils.testScriptPath = 'tools/test.dart';
+    TestingServerRunner.setPackageRootDir({'mode': 'release'});
 
-void startHttpServer(String host, int port) {
-  var basePath = TestUtils.dartDir();
-  _httpServer = new HttpServer();
-  _httpServer.onError = (e) {
-    // Consider errors in the builtin http server fatal.
-    // Intead of just throwing the exception we print
-    // a message that makes it clearer what happened.
-    print('Test http server error: $e');
-    exit(1);
-  };
-  _httpServer.defaultRequestHandler = (request, resp) {
-    var requestPath = new Path(request.path).canonicalize();
-    if (!requestPath.isAbsolute) {
-      resp.statusCode = HttpStatus.NOT_FOUND;
-      resp.outputStream.close();
-    } else {
-      var path = basePath;
-      requestPath.segments().forEach((s) => path = path.append(s));
-      var file = new File(path.toNativePath());
+    TestingServerRunner.startHttpServer('127.0.0.1',
+        port: int.parse(args['port']));
+    print(
+        'Server listening on port ${TestingServerRunner.serverList[0].port}.');
+    TestingServerRunner.startHttpServer('127.0.0.1',
+        allowedPort: TestingServerRunner.serverList[0].port, port:
+        int.parse(args['crossOriginPort']));
+    print(
+        'Server listening on port ${TestingServerRunner.serverList[1].port}.');
+  }
+}
+/**
+ * Runs a set of servers that are initialized specifically the needs for our
+ * test framework, such as dealing with package-root.
+ */
+class TestingServerRunner {
+  static List serverList = [];
+  static String _packageRootDir = null;
+
+  // Added as a getter so that the function will be called again each time the
+  // default request handler closure is executed.
+  static String get packageRootDir => _packageRootDir;
+
+  static setPackageRootDir(Map configuration) {
+    _packageRootDir = pathLib.join(TestUtils.currentWorkingDirectory.toString(),
+        TestUtils.buildDir(configuration));
+  }
+
+  static startHttpServer(String host, {int allowedPort:-1, int port: 0}) {
+    var basePath = TestUtils.dartDir().toString();
+    var httpServer = new HttpServer();
+    var packagesDirName = 'packages';
+    httpServer.onError = (e) {
+      // Consider errors in the builtin http server fatal.
+      // Intead of just throwing the exception we print
+      // a message that makes it clearer what happened.
+      print('Test http server error: $e');
+      exit(1);
+    };
+    httpServer.defaultRequestHandler = (request, resp) {
+      // Remove the initial slash in the request, since it's not an absolute

[Mads Ager (google), 2013/01/10 09:18:55]

I don't think this will work on Windows. I patched this in on my Windows machine
and something is fishy.

I don't like the mix of Path libraries in this change. It makes it very
confusing. I would really prefer if we can stick with one path library being
used. It seems simplest to stick with the dart:io one. Could you give me the
examples of stuff that you would like that doesn't work with that library?

[Bob Nystrom, 2013/01/10 19:36:43]

Any details here? If there's bugs in the path package, they will affect pub so
I'd like to know.

[Mads Ager (google), 2013/01/11 10:11:03]

This is not a problem with the pathos library but I think it is an invalid use
of it on Windows. :-)

When working with HttpRequest paths you should not use the platform path
separator. In the dart:io path library that is why there is a specific
constructor to deal with native paths. I guess in pathos you could use a builder
here that explicitly deals with '/' as the separator on all platforms?

Here is what this will do to a request path on Windows:

var requestPath = '/build/flaf.dart';
var normalized = normalize(request.path);  // 'build\flaf.dart' ... yikes
var substring = normalized.substring(1);  // 'uild\flaf.dart' ... double yikes

Because of these subtleties and differences in the libraries we should only use
one of them. There is a comment in here somewhere that Path.isAbsolute does not
work on Windows. Could you give me an example of that?

I strongly agree with the direction of this patch and I want it to land soon.
Emily, could you add a TODO and make sure to unify how we deal with paths in a
follow-up change? :-)

[Bob Nystrom, 2013/01/11 16:27:42]

Yes, that's exactly why Builder exists. Instead of calling the top level
functions, you can just do:

var builder = new pathLib.Builder(style: pathLib.Style.posix);

and then do builder.normalize() instead of pathLib.normalize();
Yeah, I discussed this a bit with Emily. Using the top level functions is the
wrong thing here because the code is dealing with URLs and not the paths for the
host filesystem.
From digging through the code, I think it may do the right thing if you're
scrupulous about using fromNative() and toNativeString()? I know I've been
burned by isAbsolute on Path before and I'm pretty good about using those (I
have the scars from failing there), but I can't recall the details now, sorry.

It may just be that I was confused by the documentation which says: "Is this
path an absolute path, beginning with a path separator?" which doesn't make
sense for a Windows path.

+      // url.
+      var requestPath = pathLib.normalize(request.path).substring(1);
+      var path = pathLib.join(basePath, requestPath);
+      var file = new File(path);
+
+      if (requestPath.toString().contains(packagesDirName)) {

[Bob Nystrom, 2013/01/10 19:36:43]

This will do the wrong thing if you have a directory with "packages" in the
middle of the name. Instead, you can do:

if (pathLib.split(requestPath).contains(packagesDirName))

[Emily Fortuna, 2013/01/11 02:57:12]

Done.

+        // Essentially implement the packages path rewriting, so we don't have
+        // to pass environment variables to the browsers.
+        requestPath = requestPath.substring(requestPath.indexOf(packagesDirName));

[Bob Nystrom, 2013/01/10 19:36:43]

Long line.

[Emily Fortuna, 2013/01/11 02:57:12]

Done.

+        path = pathLib.join(packageRootDir, requestPath);
+        file = new File(path);
+      }
       file.exists().then((exists) {
         if (exists) {
-          // Allow loading from localhost in browsers.
-          resp.headers.set("Access-Control-Allow-Origin", "*");
+          if (allowedPort != -1) {
+            // Allow loading from localhost:$allowedPort in browsers.
+            resp.headers.set("Access-Control-Allow-Origin",
+                "http://127.0.0.1:$allowedPort");
+            resp.headers.set('Access-Control-Allow-Credentials', 'true');
+          } else {
+            // No allowedPort specified. Allow from anywhere (but cross-origin
+            // requests *with credentials* will fail because you can't use "*").
+            resp.headers.set("Access-Control-Allow-Origin", "*");
+          }
+          if (path.endsWith('.html')) {
+            resp.headers.set('Content-Type', 'text/html');
+          } else if (path.endsWith('.js')) {
+            resp.headers.set('Content-Type', 'application/javascript');
+          } else if (path.endsWith('.dart')) {
+            resp.headers.set('Content-Type', 'application/dart');
+          }
           file.openInputStream().pipe(resp.outputStream);
         } else {
           resp.statusCode = HttpStatus.NOT_FOUND;
-          resp.outputStream.close();
+          try {
+            resp.outputStream.close();
+          } catch (e) {
+            if (e is StreamException) {
+              print('Test http_server error closing the response stream: $e');
+            } else {
+              throw e;
+            }
+          }
         }
       });
-    }
-  };
 
-  // Echos back the contents of the request as the response data.
-  _httpServer.addRequestHandler((req) => req.path == "/echo", (request, resp) {
-    resp.headers.set("Access-Control-Allow-Origin", "*");
+    };
 
-    request.inputStream.pipe(resp.outputStream);
-  });
+    // Echos back the contents of the request as the response data.
+    httpServer.addRequestHandler((req) => req.path == "/echo", (request, resp) {
+      resp.headers.set("Access-Control-Allow-Origin", "*");
 
-  _httpServer.listen(host, port);
+      request.inputStream.pipe(resp.outputStream);
+    });
+
+    httpServer.listen(host, port);
+    serverList.add(httpServer);
+  }
+
+  static terminateHttpServers() {
+    for (var server in serverList) server.close();
+  }
 }
-
-terminateHttpServer() {
-  if (_httpServer != null) _httpServer.close();
-}