Create generated tests inside the build directory

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 84a5b771913bb4f090f67243b8ed16cb8aca00e3..d7712bd8643b53d9c78a7c66e0a2839d9c912e25 100644
--- a/tools/testing/dart/http_server.dart
+++ b/tools/testing/dart/http_server.dart
@@ -11,6 +11,10 @@ import 'test_suite.dart';  // For TestUtils.
 // expected number of arguments, so test.dart doesn't rely on the args library?
 // See discussion on https://codereview.chromium.org/11931025/.
 import 'vendored_pkg/args/args.dart';
+import 'utils.dart';
+
+const PREFIX_BUILDDIR = 'root_build';

[ricow1, 2013/02/12 07:14:04]

how about putting these in the utils class, we use it elsewhere as well

[Emily Fortuna, 2013/02/12 18:04:09]

I know you have an explanation for how these are used down on lines 141, but can
you put some comments explaining what these constants are for exactly here when
the person opens the file?

Also, nit-picky, but why not "build_root" instead of "root_build"?

[kustermann, 2013/02/15 08:00:30]

I'm not sure what we should do. Here's my reasoning: 
These constants are part of the REST API of this server. The REST API is what
the http server provides to the outside world, so having them in
http_server.dart seems logical.

[kustermann, 2013/02/15 08:00:30]

Good idea. Done.
Actually I started with 'build_root', but having
  /root_dart
  /root_build
  /root_packages   [not yet]
seems more appealing to me than (I like the common prefix)
  /dart_root
  /build_root
  /package_root    [not yet]

+const PREFIX_DARTDIR = 'root_dart';
 
 main() {
   /** Convenience method for local testing. */
@@ -40,7 +44,11 @@ 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']);
     TestingServerRunner.startHttpServer('127.0.0.1',
         port: int.parse(args['port']));
     print('Server listening on port '
@@ -59,70 +67,122 @@ main() {
 class TestingServerRunner {
   static List serverList = [];
   static Path _packageRootDir = null;
+  static Path _buildDirectory = null;
 
   // 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;
 
   static setPackageRootDir(Map configuration) {
-    _packageRootDir = TestUtils.currentWorkingDirectory.join(
+    _packageRootDir = TestUtils.absolutePath(
+        new Path(TestUtils.buildDir(configuration)));
+  }
+
+  static setBuildDir(Map configuration) {
+    _buildDirectory = TestUtils.absolutePath(
         new Path(TestUtils.buildDir(configuration)));
   }
 
   static startHttpServer(String host, {int allowedPort:-1, int port: 0}) {
-    var basePath = TestUtils.dartDir();
     var httpServer = new HttpServer();
-    var packagesDirName = 'packages';
     httpServer.onError = (e) {
-      // TODO(ricow): Once we have a debug log we should write this out there.
-      print('Test http server error: $e');
+      DebugLogger.error('HttpServer: an error occured: $e');
     };
     httpServer.defaultRequestHandler = (request, resp) {
-      var requestPath = new Path(request.path.substring(1)).canonicalize();
-      var path = basePath.join(requestPath);
-      var file = new File(path.toNativePath());
-
-      if (requestPath.segments().contains(packagesDirName)) {
-        // Essentially implement the packages path rewriting, so we don't have
-        // to pass environment variables to the browsers.
-        var requestPathStr = requestPath.toNativePath().substring(
-            requestPath.toNativePath().indexOf(packagesDirName));
-        path = packageRootDir.append(requestPathStr);
-        file = new File(path.toNativePath());
-      }
-      file.exists().then((exists) {
-        if (exists) {
-          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');
+      void respondWithNotFound() {
+        // 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).
+        DebugLogger.warning('HttpServer: could not find file for request path: '
+                            '"${request.path}"');
+        resp.statusCode = HttpStatus.NOT_FOUND;
+        try {
+          resp.outputStream.close();
+        } catch (e) {
+          if (e is StreamException) {
+            DebugLogger.warning('HttpServer: error while closing the response '
+                                'stream: $e');
           } 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.toNativePath().endsWith('.html')) {
-            resp.headers.set('Content-Type', 'text/html');
-          } else if (path.toNativePath().endsWith('.js')) {
-            resp.headers.set('Content-Type', 'application/javascript');
-          } else if (path.toNativePath().endsWith('.dart')) {
-            resp.headers.set('Content-Type', 'application/dart');
+            throw e;
           }
-          file.openInputStream().pipe(resp.outputStream);
+        }
+      }
+
+      void respondWithFileContent(Path path, File file) {
+        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 {
-          resp.statusCode = HttpStatus.NOT_FOUND;
-          try {
-            resp.outputStream.close();
-          } catch (e) {
-            if (e is StreamException) {
-              print('Test http_server error closing the response stream: $e');
-            } else {
-              throw e;
-            }
+          // 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.filename.endsWith('.html')) {
+          resp.headers.set('Content-Type', 'text/html');
+        } else if (path.filename.endsWith('.js')) {
+          resp.headers.set('Content-Type', 'application/javascript');
+        } else if (path.filename.endsWith('.dart')) {
+          resp.headers.set('Content-Type', 'application/dart');
+        }
+        file.openInputStream().pipe(resp.outputStream);
+      }
+
+      Path getFilePathFromRequestPath(String urlRequestPath) {

[ricow1, 2013/02/12 07:14:04]

could we just move this and the other functions out of the request handler

[kustermann, 2013/02/15 08:00:30]

Done.

+        // TODO(kustermann,ricow): We could change this to the following scheme:
+        // http://host:port/root_packages/X -> $BuildDir/packages/X
+        // Issue: 8368
+
+        // NOTE: files from the dart and from the build directory are served as

[ricow1, 2013/02/12 07:14:04]

from the dart -> from the dart repository

[kustermann, 2013/02/15 08:00:30]

Done.

+        // follows:
+        // http://host:port/$PREFIX_DARTDIR/X   -> $DartDir/X
+        // http://host:port/$PREFIX_BUILDDIR/X  -> $BuildDir/X
+
+        var requestPath = new Path(urlRequestPath.substring(1)).canonicalize();
+        var pathSegments = requestPath.segments();
+        if (pathSegments.length > 1) {

[ricow1, 2013/02/12 07:14:04]

using segments here seems like a lot of work, why not just
urlRequestPath.startsWith(PREFIX_BUILDDIR)

[Emily Fortuna, 2013/02/12 18:04:09]

Also, consider using the relativeTo function from the vendored_pkg path lib
rather than rewriting yet another of your own. It's thoroughly tested.

[kustermann, 2013/02/15 08:00:30]

a) segments was used in the old code as well
('requestPath.segments().contains(packagesDirName)')
b) IMHO the code is easier to read/understand with segments

[kustermann, 2013/02/15 08:00:30]

a) I don't like using vendored_pkg: the less things we depend on, the better
b) I don't get your point. I don't see any 'relativeTo' functionality here?! 
This code does the following (simplified):
  [first_segment, remaining] = requestPath.segments()
  switch (first_segment) {
      case PREFIX_BUILDDIR: return buildDir.join(remaining);
      case PREFIX_DARTDIR:  return dartDir.join(remaining);
      default: return null;
  }
(ignoring the search for 'packages')

+          var basePath;
+          var relativePath;
+          if (pathSegments[0] == PREFIX_BUILDDIR) {
+            basePath = _buildDirectory;
+            relativePath = new Path(
+                pathSegments.getRange(1, pathSegments.length - 1).join('/'));
+          } else if (pathSegments[0] == PREFIX_DARTDIR) {
+            basePath = TestUtils.dartDir();
+            relativePath = new Path(
+                pathSegments.getRange(1, pathSegments.length - 1).join('/'));
+          }
+          var packagesDirName = 'packages';
+          var packagesIndex = pathSegments.indexOf(packagesDirName);
+          if (packagesIndex != -1 && packagesIndex < pathSegments.length - 1) {
+            var start = packagesIndex + 1;
+            var length = pathSegments.length - start;
+            basePath = _packageRootDir.append(packagesDirName);
+            relativePath = new Path(
+                pathSegments.getRange(start, length).join('/'));
+          }
+          if (basePath != null && relativePath != null) {
+            return basePath.join(relativePath);
           }
         }
-      });
+        return null;
+      }
+
+      var path = getFilePathFromRequestPath(request.path);
+      if (path != null) {
+        var file = new File.fromPath(path);
+        file.exists().then((exists) {
+          if (exists) {
+            respondWithFileContent(path, file);
+          } else {
+            respondWithNotFound();
+          }
+        });
+      } else {
+        respondWithNotFound();
+      }
     };
 
     // Echos back the contents of the request as the response data.