test.py: Support for CompileTimeError,RuntimeError,MissingRuntimeError,MissingCompiletimeError mark…

tools/testing/dart/test_runner.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.
 
 /**
  * Classes and methods for executing tests.
  *
  * This module includes:
  * - Managing parallel execution of tests, including timeout checks.
  * - Evaluating the output of each test as pass/fail/crash/timeout.
@@ skipping 361 matching lines @@
   }
 
   bool _equal(Command other) {
     return
         other is AnalysisCommand &&
         super._equal(other) &&
         flavor == other.flavor;
   }
 }
 
+class VmCommand extends Command {
+  VmCommand._(String executable,
+              List<String> arguments,
+              String configurationDir)
+      : super._("vm", executable, arguments, configurationDir);
+}
+
+class JSCommandlineCommand extends Command {
+  JSCommandlineCommand._(String displayName,
+                         String executable,
+                         List<String> arguments,
+                         String configurationDir,
+                         [Map<String, String> environmentOverrides = null])
+      : super._(displayName,
+                executable,
+                arguments,
+                configurationDir,
+                environmentOverrides);
+}
+
 class CommandBuilder {
   static final instance = new CommandBuilder._();
 
   final _cachedCommands = new Map<Command, Command>();
 
   CommandBuilder._();
 
   ContentShellCommand getContentShellCommand(String executable,
                                              String htmlFile,
                                              List<String> options,
@@ skipping 41 matching lines @@
   }
 
   AnalysisCommand getAnalysisCommand(
       String displayName, executable, arguments, String configurationDir,
       {String flavor: 'dartanalyzer'}) {
     var command = new AnalysisCommand._(
         flavor, displayName, executable, arguments, configurationDir);
     return _getUniqueCommand(command);
   }
 
+  VmCommand getVmCommand(String executable,
+                         List<String> arguments,
+                         String configurationDir) {
+    var command = new VmCommand._(executable, arguments, configurationDir);
+    return _getUniqueCommand(command);
+  }
+
+  Command getJSCommandlineCommand(String displayName, executable, arguments,
+                     String configurationDir, [environment = null]) {
+    var command = new JSCommandlineCommand._(displayName, executable, arguments,
+                                             configurationDir, environment);
+    return _getUniqueCommand(command);
+  }
+
   Command getCommand(String displayName, executable, arguments,
                      String configurationDir, [environment = null]) {
     var command = new Command._(displayName, executable, arguments,
                                 configurationDir, environment);
     return _getUniqueCommand(command);
   }
 
   Command _getUniqueCommand(Command command) {
     // All Command classes have hashCode/operator==, so we check if this command
     // has already been build, if so we return the cached one, otherwise we
@@ skipping 30 matching lines @@
    * Dart2js tests have two commands, one to compile the source and another
    * to execute it. Some isolate tests might even have three, if they require
    * compiling multiple sources that are run in isolation.
    */
   List<Command> commands;
   Map<Command, CommandOutput> commandOutputs = new Map<Command,CommandOutput>();
 
   Map configuration;
   String displayName;
   bool isNegative;
-  Set<String> expectedOutcomes;
+  Set<Expectation> expectedOutcomes;
   TestInformation info;
 
   TestCase(this.displayName,
            this.commands,
            this.configuration,
            this.expectedOutcomes,
            {this.isNegative: false,
             this.info: null}) {
     if (!isNegative) {
       this.isNegative = displayName.contains("negative_test");
     }
   }
 
   bool get unexpectedOutput {
-    return !expectedOutcomes.contains(lastCommandOutput.result(this));
+    var outcome = lastCommandOutput.result(this);
+    return !expectedOutcomes.any((expectation) {
+      var result = outcome.canBeOutcomeOf(expectation);
+      if (result) {
+        if (outcome != expectation) {
+          // FIXME/TODO(kustermann): We should bubble this up a bit smarter
+          //print("Could narrow Expected: $expectation to $outcome");

[ricow1, 2013/09/23 12:07:32]

you don't do anything + no FIXME

[kustermann, 2013/09/23 15:31:34]

This was actually intentional -- I left this comment to remind me about it (I
can't do it in this CL).

Though I can remove the comment.

+        }
+      }
+      return result;
+    });
   }
 
-  String get result => lastCommandOutput.result(this);
+  Expectation get result => lastCommandOutput.result(this);
 
   CommandOutput get lastCommandOutput {
     if (commandOutputs.length == 0) {
       throw new Exception("CommandOutputs is empty, maybe no command was run? ("
                           "displayName: '$displayName', "
                           "configurationString: '$configurationString')");
     }
     return commandOutputs[commands[commandOutputs.length - 1]];
   }
 
   int get timeout {
-    if (expectedOutcomes.contains(SLOW)) {
+    if (expectedOutcomes.contains(Expectation.SLOW)) {
       return configuration['timeout'] * SLOW_TIMEOUT_MULTIPLIER;
     } else {
       return configuration['timeout'];
     }
   }
 
   String get configurationString {
     final compiler = configuration['compiler'];
     final runtime = configuration['runtime'];
     final mode = configuration['mode'];
     final arch = configuration['arch'];
     final checked = configuration['checked'] ? '-checked' : '';
     return "$compiler-$runtime$checked ${mode}_$arch";
   }
 
   List<String> get batchTestArguments => commands.last.arguments;
 
   bool get usesWebDriver => TestUtils.usesWebDriver(configuration['runtime']);
 
   bool get isFlaky {
-      if (expectedOutcomes.contains(SKIP) ||
-          expectedOutcomes.contains(SKIP_BY_DESIGN)) {
+      if (expectedOutcomes.contains(Expectation.SKIP) ||
+          expectedOutcomes.contains(Expectation.SKIP_BY_DESIGN)) {
         return false;
       }
 
-      var flags = new Set.from(expectedOutcomes);
-      flags..remove(OK)
-           ..remove(SLOW);
-      return flags.length > 1;
+      return expectedOutcomes
+        .where((expectation) => !expectation.isMetaExpectation).length > 1;
   }
 
   bool get isFinished {
     return !lastCommandOutput.successful ||
            commands.length == commandOutputs.length;
   }
 }
 
 
 /**
@@ skipping 15 matching lines @@
 
 /**
  * CommandOutput records the output of a completed command: the process's exit
  * code, the standard output and standard error, whether the process timed out,
  * and the time the process took to run.  It also contains a pointer to the
  * [TestCase] this is the output of.
  */
 abstract class CommandOutput {
   Command get command;
 
-  String result(TestCase testCase);
+  Expectation result(TestCase testCase);
 
   bool get hasCrashed;
 
   bool get hasTimedOut;
 
   bool didFail(testcase);
 
   bool hasFailed(TestCase testCase);
 
   bool get canRunDependendCommands;
@@ skipping 34 matching lines @@
   CommandOutputImpl(Command this.command,
                     int this.exitCode,
                     bool this.timedOut,
                     List<int> this.stdout,
                     List<int> this.stderr,
                     Duration this.time,
                     bool this.compilationSkipped) {
     diagnostics = [];
   }
 
-  String result(TestCase testCase) => hasCrashed ? CRASH :
-      (hasTimedOut ? TIMEOUT : (hasFailed(testCase) ? FAIL : PASS));
+  Expectation result(TestCase testCase) {
+    if (hasCrashed) return Expectation.CRASH;
+    if (hasTimedOut) return Expectation.TIMEOUT;
+    return hasFailed(testCase) ? Expectation.FAIL : Expectation.PASS;
+  }
 
   bool get hasCrashed {
     // The Java dartc runner and dart2js exits with code 253 in case
     // of unhandled exceptions.
     if (exitCode == 253) return true;
     if (io.Platform.operatingSystem == 'windows') {
       // The VM uses std::abort to terminate on asserts.
       // std::abort terminates with exit code 3 on Windows.
       if (exitCode == 3) {
         return !timedOut;
@@ skipping 25 matching lines @@
     return !hasTimedOut && exitCode == 0;
   }
 
   bool get successful {
     // FIXME(kustermann): We may need to change this
     return !hasTimedOut && exitCode == 0;
   }
 
   // Reverse result of a negative test.
   bool hasFailed(TestCase testCase) {
-    // FIXME(kustermann): this is a hack, remove it
-    bool isCompilationCommand = testCase.commands.first == command
-        && testCase.commands.length > 1;
-    if (isCompilationCommand &&
-        testCase.info != null && testCase.info.hasRuntimeError) {
-      return exitCode != 0;
-    }
     return testCase.isNegative ? !didFail(testCase) : didFail(testCase);
   }
 }
 
 class BrowserCommandOutputImpl extends CommandOutputImpl {
   bool _failedBecauseOfMissingXDisplay;
 
   BrowserCommandOutputImpl(
       command,
       exitCode,
@@ skipping 10 matching lines @@
           time,
           compilationSkipped) {
     _failedBecauseOfMissingXDisplay = _didFailBecauseOfMissingXDisplay();
     if (_failedBecauseOfMissingXDisplay) {
       DebugLogger.warning("Warning: Test failure because of missing XDisplay");
       // If we get the X server error, or DRT crashes with a core dump, retry
       // the test.
     }
   }
 
+  Expectation result(TestCase testCase) {
+    // Handle crashes and timeouts first
+    if (hasCrashed) return Expectation.CRASH;
+    if (hasTimedOut) return Expectation.TIMEOUT;
+
+    var outcome = _getOutcome();
+
+    if (testCase.info != null && testCase.info.hasRuntimeError) {
+      if (!outcome.canBeOutcomeOf(Expectation.RUNTIME_ERROR)) {
+        return Expectation.MISSING_RUNTIME_ERROR;
+      }
+    }
+
+    if (testCase.isNegative) {
+      if (outcome.canBeOutcomeOf(Expectation.FAIL)) return Expectation.PASS;
+      return Expectation.FAIL;
+    }
+    return outcome;
+  }
+
   bool get successful => canRunDependendCommands;
 
   bool get canRunDependendCommands {
     // We cannot rely on the exit code of content_shell as a method to determine
     // if we were successful or not.
     return super.canRunDependendCommands && !didFail(null);
   }
 
-  bool didFail(TestCase _) {
+  Expectation _getOutcome() {
     if (_failedBecauseOfMissingXDisplay) {
-      return true;
+      return Expectation.FAIL;
     }
 
     if (command.expectedOutputFile != null) {
       // We are either doing a pixel test or a layout test with content shell
-      return _failedBecauseOfUnexpectedDRTOutput;
+      if (_failedBecauseOfUnexpectedDRTOutput) {
+        return Expectation.FAIL;
+      }
     }
-    return _browserTestFailure;
+    if (_browserTestFailure) {
+      return Expectation.RUNTIME_ERROR;
+    }
+    return Expectation.PASS;
   }
 
   bool _didFailBecauseOfMissingXDisplay() {
     // Browser case:
     // If the browser test failed, it may have been because content shell
     // and the virtual framebuffer X server didn't hook up, or it crashed with
     // a core dump. Sometimes content shell crashes after it has set the stdout
     // to PASS, so we have to do this check first.
     var stderrLines = decodeUtf8(super.stderr).split("\n");
     for (String line in stderrLines) {
@@ skipping 281 matching lines @@
         field = new StringBuffer();
         continue;
       }
       field.write(c);
     }
     result.add(field.toString());
     return result;
   }
 }
 
+class VmCommandOutputImpl extends CommandOutputImpl {
+  static const DART_VM_EXITCODE_COMPILE_TIME_ERROR = 254;
+  static const DART_VM_EXITCODE_UNCAUGHT_EXCEPTION = 255;
+
+  VmCommandOutputImpl(Command command, int exitCode, bool timedOut,
+      List<int> stdout, List<int> stderr, Duration time)
+      : super(command, exitCode, timedOut, stdout, stderr, time, false);
+
+  Expectation result(TestCase testCase) {
+    // Handle crashes and timeouts first
+    if (hasCrashed) return Expectation.CRASH;
+    if (hasTimedOut) return Expectation.TIMEOUT;
+
+    // Multitests are handled specially
+    if (testCase.info != null) {
+      if (testCase.info.hasCompileError) {
+        if (exitCode == DART_VM_EXITCODE_COMPILE_TIME_ERROR) {
+          return Expectation.PASS;
+        }
+
+        // We're not as strict, if the exitCode indicated an uncaught exception
+        // we say it passed nonetheless
+        // TODO(kustermann): As soon as the VM team makes sure we get correct
+        // exit codes, we should remove this.
+        if (exitCode == DART_VM_EXITCODE_UNCAUGHT_EXCEPTION) {
+          return Expectation.PASS;
+        }
+
+        return Expectation.MISSING_COMPILETIME_ERROR;
+      }
+      if (testCase.info.hasRuntimeError) {
+        // FIXME(kustermann): Do we consider a "runtimeError" only an uncaught

[ricow1, 2013/09/23 12:07:32]

FIXME->TODO

[kustermann, 2013/09/23 15:31:34]

Done.

+        // exception or does any nonzero exit code fullfil this requirement?
+        if (exitCode != 0) {

[ricow1, 2013/09/23 12:07:32]

well I guess at least DART_VM_EXITCODE_COMPILE_TIME_ERROR should not be allowed
here, Ivan can probably give additional info here

[kustermann, 2013/09/23 15:31:34]

I can't do that because some tests will start failing then.

I'll send the VM guys a patch how to enable it (and a list of affected tests),
then they can fix the exitcode issues.

+          return Expectation.PASS;
+        }
+        return Expectation.MISSING_RUNTIME_ERROR;
+      }
+    }
+
+    // The actual outcome depends on the exitCode
+    Expectation outcome;
+    if (exitCode == DART_VM_EXITCODE_COMPILE_TIME_ERROR) {
+      outcome = Expectation.COMPILETIME_ERROR;
+    } else if (exitCode == DART_VM_EXITCODE_UNCAUGHT_EXCEPTION) {
+      outcome = Expectation.RUNTIME_ERROR;
+    } else if (exitCode != 0) {
+      // This is a general fail, in case we get an unknown nonzero exitcode.
+      outcome = Expectation.FAIL;
+    } else {
+      outcome = Expectation.PASS;
+    }
+
+    // In case it's a negative test, reverse the result
+    if (outcome.canBeOutcomeOf(Expectation.FAIL)) {
+      if (testCase.isNegative) outcome = Expectation.PASS;
+    } else {
+      if (testCase.isNegative) outcome = Expectation.FAIL;
+    }
+    return outcome;
+  }
+}
+
+class CompilationCommandOutputImpl extends CommandOutputImpl {
+  static const DART2JS_EXITCODE_CRASH = 253;
+
+  CompilationCommandOutputImpl(Command command, int exitCode, bool timedOut,
+      List<int> stdout, List<int> stderr, Duration time)
+      : super(command, exitCode, timedOut, stdout, stderr, time, false);
+
+  Expectation result(TestCase testCase) {
+    // Handle general crash/timeout detection.
+    if (hasCrashed) return Expectation.CRASH;
+    if (hasTimedOut) return Expectation.TIMEOUT;
+
+    // Handle dart2js/dart2dart specific crash detection
+    if (exitCode == DART2JS_EXITCODE_CRASH ||
+        exitCode == VmCommandOutputImpl.DART_VM_EXITCODE_COMPILE_TIME_ERROR ||

[ricow1, 2013/09/23 12:07:32]

If seems a little strange to reference constants in another implementation class
here, could we move them out?

[kustermann, 2013/09/23 15:31:34]

IMHO it's not strange at all. The VmCommandOutputImpl is the class responsible
for determining what the outcome of running a VM was. So having the constants
there seems natural.

Why should we not keep it this way?

+        exitCode == VmCommandOutputImpl.DART_VM_EXITCODE_UNCAUGHT_EXCEPTION) {
+      return Expectation.CRASH;
+    }
+
+    // Multitests are handled specially
+    if (testCase.info != null) {
+      if (testCase.info.hasCompileError) {
+        // Nonzero exit code of the compiler means compilation failed
+        // FIXME(kustermann): Do we have a special exit code in that case???

[ricow1, 2013/09/23 12:07:32]

fixme->todo

[kustermann, 2013/09/23 15:31:34]

Done.

+        if (exitCode != 0) {
+          return Expectation.PASS;
+        }
+        return Expectation.MISSING_COMPILETIME_ERROR;
+      }
+
+      // FIXME(kustermann): this is a hack, remove it

[ricow1, 2013/09/23 12:07:32]

fixme -> todo

[kustermann, 2013/09/23 15:31:34]

Done.

+      if (testCase.info.hasRuntimeError && testCase.commands.length > 1) {
+        // We expected to run the test, but we got an compile time error.
+        // If the compilation succeeded, we wouldn't be in here!
+        assert(exitCode != 0);
+        return Expectation.COMPILETIME_ERROR;
+      }
+    }
+
+    Expectation outcome =
+        exitCode == 0 ? Expectation.PASS : Expectation.COMPILETIME_ERROR;
+    // In case it's a negative test, reverse the result
+    if (outcome.canBeOutcomeOf(Expectation.FAIL)) {

[ricow1, 2013/09/23 12:07:32]

this code is duplicated here, above and below, could we extract it and just call
that function

[kustermann, 2013/09/23 15:31:34]

I thought about this as well. I moved it now up to the superclass.

+      if (testCase.isNegative) outcome = Expectation.PASS;
+    } else {
+      if (testCase.isNegative) outcome = Expectation.FAIL;
+    }
+    return outcome;
+  }
+}
+
+class JsCommandlineOutputImpl extends CommandOutputImpl {
+  JsCommandlineOutputImpl(Command command, int exitCode, bool timedOut,
+      List<int> stdout, List<int> stderr, Duration time)
+      : super(command, exitCode, timedOut, stdout, stderr, time, false);
+
+  Expectation result(TestCase testCase) {
+    // Handle crashes and timeouts first
+    if (hasCrashed) return Expectation.CRASH;
+    if (hasTimedOut) return Expectation.TIMEOUT;
+
+    if (testCase.info != null && testCase.info.hasRuntimeError) {
+      if (exitCode != 0) return Expectation.PASS;
+      return Expectation.MISSING_RUNTIME_ERROR;
+    }
+
+    var outcome = exitCode == 0 ? Expectation.PASS : Expectation.RUNTIME_ERROR;
+    // Handle negative tests
+    if (testCase.isNegative) {
+      if (outcome.canBeOutcomeOf(Expectation.FAIL)) return Expectation.PASS;
+      return Expectation.FAIL;
+    }
+    return outcome;
+  }
+}
 
 CommandOutput createCommandOutput(Command command,
                                   int exitCode,
                                   bool timedOut,
                                   List<int> stdout,
                                   List<int> stderr,
                                   Duration time,
                                   bool compilationSkipped) {
   if (command is ContentShellCommand) {
     return new BrowserCommandOutputImpl(
         command, exitCode, timedOut, stdout, stderr,
         time, compilationSkipped);
   } else if (command is BrowserTestCommand) {
     return new HTMLBrowserCommandOutputImpl(
         command, exitCode, timedOut, stdout, stderr,
         time, compilationSkipped);
   } else if (command is SeleniumTestCommand) {
     return new BrowserCommandOutputImpl(
         command, exitCode, timedOut, stdout, stderr,
         time, compilationSkipped);
   } else if (command is AnalysisCommand) {
     return new AnalysisCommandOutputImpl(
         command, exitCode, timedOut, stdout, stderr,
         time, compilationSkipped);
+  } else if (command is VmCommand) {
+    return new VmCommandOutputImpl(
+        command, exitCode, timedOut, stdout, stderr, time);
+  } else if (command is CompilationCommand) {
+    return new CompilationCommandOutputImpl(
+        command, exitCode, timedOut, stdout, stderr, time);
+  } else if (command is JSCommandlineCommand) {
+    return new JsCommandlineOutputImpl(
+        command, exitCode, timedOut, stdout, stderr, time);
   }
+
   return new CommandOutputImpl(
       command, exitCode, timedOut, stdout, stderr,
       time, compilationSkipped);
 }
 
 
 /** Modifies the --timeout=XX parameter passed to run_selenium.py */
 List<String> _modifySeleniumTimeout(List<String> arguments, int timeout) {
   return arguments.map((argument) {
     if (argument.startsWith('--timeout=')) {
@@ skipping 1075 matching lines @@
     }
   }
 
   void eventAllTestsDone() {
     for (var listener in _eventListener) {
       listener.allDone();
     }
     _allDone();
   }
 }