Clean up test.dart by merging dartium and chromium test case generation.

tools/testing/dart/test_suite.dart

 // Copyright (c) 2011, 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("test_suite");
 
 #import("status_file_parser.dart");
 #import("test_runner.dart");
 #import("multitest.dart");
 
@@ skipping 233 matching lines @@
       start = filename.indexOf(directoryPath);
       testName = filename.substring(start + directoryPath.length + 1,
                                     filename.length);
       if (configuration['component'] != 'dartc') {
         if (testName.endsWith('.dart')) {
           testName = testName.substring(0, testName.length - 5);
         }
       }
     }
     Set<String> expectations = testExpectations.expectations(testName);
-    if (configuration["report"]) {
+    if (configuration['report']) {
       // Tests with multiple VMOptions are counted more than once.
       for (var dummy in optionsFromFile["vmOptions"]) {
         SummaryReport.add(expectations);
       }
     }
     if (expectations.contains(SKIP)) return;
 
     switch (configuration['component']) {
       case 'dartium':
-        enqueueDartiumTest(filename, testName, optionsFromFile,
-                           expectations, isNegative);
-        break;
       case 'chromium':
       case 'frogium':
-        enqueueChromiumTest(filename, testName, optionsFromFile,
-                            expectations, isNegative);
+          enqueueBrowserTest(filename, testName,optionsFromFile,

[Bill Hesse, 2012/01/05 09:28:18]

Oops - missing space.

+                             expectations, isNegative);
         break;
       default:
         // Only dartc supports fatal type errors. Enable fatal type
         // errors with a flag and treat tests that have fatal type
         // errors as negative.
         var enableFatalTypeErrors =
             (info.hasFatalTypeErrors && configuration['component'] == 'dartc');
         var argumentLists = argumentListsFromFile(filename,
                                                   optionsFromFile,
                                                   enableFatalTypeErrors);
@@ skipping 45 matching lines @@
       DoMultitest(filename,
                   TestUtils.outputDir(configuration),
                   directoryPath,
                   createTestCase,
                   testGeneratorDone);
     } else {
       createTestCase(filename, optionsFromFile['isNegative']);
     }
   }
         
-
-  void enqueueDartiumTest(String filename,
+  void enqueueBrowserTest(String filename,
                           String testName,
                           Map optionsFromFile,
                           Set<String> expectations,
                           bool isNegative) {
-    // TODO(whesse): Merge with enqueueChromiumTest, using mainly
-    // enqueueChromiumTest's code and design.
     if (optionsFromFile['isMultitest']) return;
     bool isWebTest = optionsFromFile['containsDomImport'];
     bool isLibraryDefinition = optionsFromFile['isLibraryDefinition'];
-    if (!isLibraryDefinition && optionsFromFile['containsSourceOrImport']) {
-      print('Warning for $filename: Browser tests require #library ' +
-            'in any file that uses #import or #source');
-    }      
-
-    String tempDirTemplate = '${TestUtils.buildDir(configuration)}/tmp';
-    Directory tempDir = new Directory(tempDirTemplate);
-    tempDir.createTempSync();
-
-    String dartTestFilename = new File(filename).fullPathSync();
-    String dartWrapperFilename;
-    String scriptPath;
-    if (isWebTest) {
-      scriptPath = 'file://$dartTestFilename';
-    } else {
-      dartWrapperFilename = '${tempDir.path}/test.dart';
-      scriptPath = '../../../$dartWrapperFilename';
-      // test.dart will import the dart test directly, if it is a library,
-      // or indirectly through test_as_library.dart, if it is not.
-      String dartLibraryFilename;
-      if (isLibraryDefinition) {
-        dartLibraryFilename = dartTestFilename;
-      } else {
-        dartLibraryFilename = 'test_as_library.dart';
-        File file = new File('${tempDir.path}/$dartLibraryFilename');
-        RandomAccessFile dartLibrary = file.openSync(FileMode.WRITE);
-        dartLibrary.writeStringSync(WrapDartTestInLibrary(dartTestFilename));
-        dartLibrary.closeSync();
-      }
-
-      File file = new File(dartWrapperFilename);
-      RandomAccessFile dartWrapper = file.openSync(FileMode.WRITE);
-      dartWrapper.writeStringSync(DartTestWrapper(
-          'dart:dom',
-          '../../../tests/isolate/src/TestFramework.dart',
-          dartLibraryFilename));
-      dartWrapper.closeSync();
-    }
-    // Create the HTML file for the test.
-    // NOTE: This must be 3 directories below the dart root, due to test
-    // client/samples/dartcombat containing a relative path to its .css file.
-    File htmlTestBase = new File('${tempDir.path}/${getHtmlName(filename)}');
-    RandomAccessFile htmlTest = htmlTestBase.openSync(FileMode.WRITE);
-    htmlTest.writeStringSync(GetHtmlContents(
-        filename,
-        '../../../client/testing/unittest/test_controller.js',
-        scriptType,
-        scriptPath));
-    htmlTest.closeSync();
-
-    for (var vmOptions in optionsFromFile["vmOptions"]) {
-      var drtFlags = ['--no-timeout'];
-      var dartFlags = ['--enable_asserts', '--enable_type_checks'];
-      dartFlags.addAll(vmOptions);
-      drtFlags.add('--dart-flags=${Strings.join(dartFlags, " ")}');
-      var args = drtFlags;
-      args.add(htmlTestBase.fullPathSync());
-
-      // Create BrowserTestCase and queue it.
-      var testCase = new BrowserTestCase(
-          testName,
-          null,
-          null,
-          dumpRenderTreeFilename,
-          args,
-          configuration,
-          completeHandler,
-          expectations, optionsFromFile['isNegative']);
-      doTest(testCase);
-    }        
-  }
-
-  void enqueueChromiumTest(String filename,
-                           String testName,
-                           Map optionsFromFile,
-                           Set<String> expectations,
-                           bool isNegative) {
-    if (optionsFromFile['isMultitest']) return;
-    bool isWebTest = optionsFromFile['containsDomImport'];
-    bool isLibraryDefinition = optionsFromFile['isLibraryDefinition'];
     if (!isLibraryDefinition && optionsFromFile['containsSourceOrImport']) {
       print('Warning for $filename: Browser tests require #library ' +
             'in any file that uses #import or #source');
     }      
 
+    final String component = configuration['component'];
+    final String testPath = new File(filename).fullPathSync();
     String dartDir = new File('.').fullPathSync();
-    String buildDir = TestUtils.buildDir(configuration);
-    String testPath = new File(filename).fullPathSync();
-    String outputDirBase = '$dartDir/$buildDir/generated_tests/chromium';
-
-    Expect.isTrue(testPath.startsWith(dartDir));
-    String testRelativePath = testPath.substring(dartDir.length + 1);
-    String testNameBase;
-    String testRelativeDir;
-    String testRelativeDirFlattened;
-
-    int start = testRelativePath.lastIndexOf('src' + pathSeparator);
-    if (start != -1) {
-      Expect.isTrue(testRelativePath.endsWith('.dart'));
-      testNameBase =
-          testRelativePath.substring(start + 4, testRelativePath.length - 5);
-      testRelativeDir = testRelativePath.substring(0, start - 1);
-      testRelativeDirFlattened = testRelativeDir.replaceAll('/', '_');
-    } else {
-      Expect.isTrue(testRelativePath.endsWith('_tests.dart'));
-      start = testRelativePath.lastIndexOf(pathSeparator);
-      testNameBase =
-          testRelativePath.substring(start + 1, testRelativePath.length - 11);
+    if (!testPath.startsWith(dartDir)) {
+      dartDir = new File('..').fullPathSync();
+      if (!testPath.startsWith(dartDir)) {
+        print('Run test.dart from the dart directory or' +
+              ' an immediate subdirectory only.');
+        Expect.fail('Could not find top level dart directory.');
+      }
     }
 
-    if (!new Directory('$dartDir/$buildDir/generated_tests').existsSync()) {
-      new Directory('$dartDir/$buildDir/generated_tests').createSync();
-    }
-    if (!new Directory(outputDirBase).existsSync()) {
-      new Directory(outputDirBase).createSync();
-    }
-    Directory tempDir = new Directory(
-        '$outputDirBase/${testRelativeDirFlattened}_$testNameBase');
-    if (!tempDir.existsSync()) {
-      tempDir.createSync();
+    // Create a directory for the generated test.  Drop the path to the
+    // dart checkout and the final ".dart" from the test path, and replace
+    // all path separators with underscores.
+    // All variables are block local, except tempDir.

[Mads Ager (google), 2012/01/04 22:03:23]

I'm not sure there is value in the extra scope here? Maybe you should be
extracting things into separate functions if you feel this is getting big enough
that you want to do this?

[Bill Hesse, 2012/01/05 09:28:18]

Moved to a separate function.

On 2012/01/04 22:03:23, Mads Ager wrote:

+    Directory tempDir;
+    {
+      String testUniqueName =
+          testPath.substring(dartDir.length + 1, testPath.length - 5);
+      testUniqueName = testUniqueName.replaceAll('/', '_');
+      // Create '[build dir]/generated_tests/$component/$testUniqueName',
+      // including any intermediate directories that don't exist.
+      var generatedTestPath = ['generated_tests',
+                               component,
+                               testUniqueName];
+      String tempDirPath =
+          new File(TestUtils.buildDir(configuration)).fullPathSync();
+      for (String subdirectory in generatedTestPath) {
+        tempDirPath = '$tempDirPath/$subdirectory';
+        tempDir = new Directory(tempDirPath);
+        if (!tempDir.existsSync()) {
+          tempDir.createSync();
+        }
+      }
     }
 
     String dartWrapperFilename = '${tempDir.path}/test.dart';
     String compiledDartWrapperFilename = '${tempDir.path}/test.js';
-    String domLibraryImport = 'dart:dom';
-    if (configuration['component'] == 'chromium') {
-      domLibraryImport =
-          '$dartDir/client/testing/unittest/dom_for_unittest.dart';
-    }
+    String domLibraryImport = (component == 'chromium') ?
+        '$dartDir/client/testing/unittest/dom_for_unittest.dart' : 'dart:dom';
 
-    File htmlTestBase;
+    String htmlPath = '${tempDir.path}/test.html';
     if (!isWebTest) {
       // test.dart will import the dart test directly, if it is a library,
       // or indirectly through test_as_library.dart, if it is not.
       String dartLibraryFilename;
       if (isLibraryDefinition) {
         dartLibraryFilename = testPath;
       } else {
         dartLibraryFilename = 'test_as_library.dart';
         File file = new File('${tempDir.path}/$dartLibraryFilename');
         RandomAccessFile dartLibrary = file.openSync(FileMode.WRITE);
         dartLibrary.writeStringSync(WrapDartTestInLibrary(testPath));
         dartLibrary.closeSync();
       }  
       
       File file = new File(dartWrapperFilename);
       RandomAccessFile dartWrapper = file.openSync(FileMode.WRITE);
       dartWrapper.writeStringSync(DartTestWrapper(
           domLibraryImport,
           '$dartDir/tests/isolate/src/TestFramework.dart',
           dartLibraryFilename));
       dartWrapper.closeSync();
-      htmlTestBase = new File('${tempDir.path}/${getHtmlName(filename)}');
     } else {
       dartWrapperFilename = testPath;
       // TODO(whesse): Once test.py is retired, adjust the relative path in
       // the client/samples/dartcombat test to its css file, remove the
       // "../../" from this path, and move this out of the isWebTest guard.
-      htmlTestBase = new File('${tempDir.path}/../../${getHtmlName(filename)}');
+      // Also remove getHtmlName, and just use test.html.
+      htmlPath = '${tempDir.path}/../../${getHtmlName(filename)}';
     }
+    final String scriptPath = (component == 'dartium') ?
+        dartWrapperFilename : compiledDartWrapperFilename;
     // Create the HTML file for the test.
-    RandomAccessFile htmlTest = htmlTestBase.openSync(FileMode.WRITE);
+    RandomAccessFile htmlTest = new File(htmlPath).openSync(FileMode.WRITE);
     htmlTest.writeStringSync(GetHtmlContents(
         filename,
         '$dartDir/client/testing/unittest/test_controller.js',
         scriptType,
-        compiledDartWrapperFilename));
+        scriptPath));
     htmlTest.closeSync();
 
-    for (var vmOptions in optionsFromFile["vmOptions"]) {
+    for (var vmOptions in optionsFromFile['vmOptions']) {
       List<String> compilerArgs;
       String compilerExecutable = TestUtils.compilerPath(configuration);
-      switch (configuration['component']) {
+      switch (component) {
         case 'chromium':
           compilerArgs = ['--work', tempDir.path];
           if (configuration['mode'] ==  'release') {
             compilerArgs.add('--optimize');
           }
           compilerArgs.addAll(vmOptions);
           compilerArgs.add('--ignore-unrecognized-flags');
           compilerArgs.add('--out');
           compilerArgs.add(compiledDartWrapperFilename);
           compilerArgs.add(dartWrapperFilename);
           // TODO(whesse): Add --fatal-type-errors if needed.
           break;
         case 'frogium':
           compilerArgs = ['--libdir=$dartDir/frog/lib',
                           '--compile-only',
                           '--out=$compiledDartWrapperFilename'];
           compilerArgs.addAll(vmOptions);
           compilerArgs.add(dartWrapperFilename);
           break;
+        case 'dartium':
+          // No compilation phase.
+          compilerExecutable = null;
+          compilerArgs = null;
+          break;
         default:
-          Expect.fail('unimplemented component ${configuration['component']}');
+          Expect.fail('unimplemented component $component');
       }
 
       var args = ['--no-timeout'];
-      args.add(htmlTestBase.fullPathSync());
+      if (component == 'dartium') {
+        var dartFlags = ['--enable_asserts', '--enable_type_checks'];
+        dartFlags.addAll(vmOptions);
+        args.add('--dart-flags=${Strings.join(dartFlags, " ")}');
+      }
+      args.add(htmlPath);
 
       // Create BrowserTestCase and queue it.
       var testCase = new BrowserTestCase(
           testName,
           compilerExecutable,
           compilerArgs,
-          dumpRenderTreeFilename,
+          getFilename(dumpRenderTreeFilename),
           args,
           configuration,
           completeHandler,
           expectations,
           optionsFromFile['isNegative']);
       doTest(testCase);
     }        
   }
 
   String get scriptType() {
@@ skipping 378 matching lines @@
 
   static String dartShellFileName(Map configuration) {
     var name = '${buildDir(configuration)}/${executableName(configuration)}';
     if (!(new File(name)).existsSync()) {
       throw "Executable '$name' does not exist";
     }
     return name;
   }
 
   static String compilerPath(Map configuration) {
+    if (configuration['component'] == 'dartium') {
+      return null;  // No separate compiler for dartium tests.
+    }
     var name = '${buildDir(configuration)}/${compilerName(configuration)}';
     if (!(new File(name)).existsSync()) {
       throw "Executable '$name' does not exist";
     }
     return name;
   }
 
   static String outputDir(Map configuration) {
     var outputDir = '';
     var system = configuration['system'];
@@ skipping 76 matching lines @@
  * $noCrash tests are expected to be flaky but not crash
  * $pass tests are expected to pass
  * $failOk tests are expected to fail that we won't fix
  * $fail tests are expected to fail that we should fix
  * $crash tests are expected to crash that we should fix
  * $timeout tests are allowed to timeout\
 """;
     print(report);
   }
 }