Create generated tests inside the build directory

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.
 // TODO(efortuna): Rewrite to not use the args library and simply take an
 // 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. */
   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.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.');
   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 = new Path(new Options().script)
         .directoryPath
         .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 '
           '${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 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;
 
   // 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');
-          }
-          file.openInputStream().pipe(resp.outputStream);
-        } 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;
-            }
+            throw e;
           }
         }
-      });
+      }
+
+      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 {
+          // 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.
     httpServer.addRequestHandler((req) => req.path == "/echo", (request, resp) {
       resp.headers.set("Access-Control-Allow-Origin", "*");
 
       request.inputStream.pipe(resp.outputStream);
     });
 
     httpServer.listen(host, port);
     serverList.add(httpServer);
   }
 
   static terminateHttpServers() {
     for (var server in serverList) server.close();
   }
 }