Test failures coming out as runtime errors (and maybe timeouts)

[alan-knight]

I'm seeing some bot logs where it appears that a failure in the DateTime tests (due to known bug 15560 which causes unreliable DateTime creation) but in e.g.
http://chromegw.corp.google.com/i/client.dart/builders/dartium-lucid64-full-be/builds/3045/steps/drt_core_checked_tests/logs/stdio

it's coming out as a runtime error rather than a test failure. There's also a recently opened bug about these tests timing out sometimes on Dartium, ( https://code.google.com/p/dart/issues/detail?id=19544 ) which might also be related.

[mkustermann]

A while ago, the dart2js team requested us to distinguish between different kinds of test failures. In particular they wanted to distinguish between CompileTimeError's and RuntimeError's.

We've implemented that distinction: A test outcome can be CompileTimeError or RuntimeError. Both are a "subclass" of Fail. Marking a test as failing in the status file will cover both, compile-time and runtime errors. But you can be more specific, to make it clear whether it happened at runtime or not.

In the specific test failure you mentioned, you'll see at the end of the json log, that there was an error reported via the "window.onerror" handler. This makes it a runtime error:
....
CONSOLE MESSAGE: line 78: {
  "type": "window_onerror",
  "value": "window.onerror called: \n\nFIXME:0:\nException: Exception: Some tests failed.\n\n",
  "timestamp": 2.804
}

Since this is rather a feature then a bug, I'll close this as invalid. In case you ment something else, feel free to re-open.

[I don't know why the tests are sometimes timing out, but this is tracked in issue #19544 I guess.]

---

cc @ricowind.
cc @whesse.
Added Invalid label.

[alan-knight]

Those don't seem like subclasses of FAIL to me. I would expect the categorization to be

FAIL - test program ran, the assertions that it checked were not satisfied
RUNTIME ERROR - the program threw an unhandled exception and terminated
COMPILE ERROR - the program could not be compiled

What is the benefit of having these as subclasses? It seems like a very bad idea to mark a test as FAIL, when this would mask, for example, that a failure to produce the correct results had turned into the inability to compile the test at all. And when tests do fail, it seems unnecessarily hard to read the output.

[mkustermann]

We used to have only "Fail" as outcomes of tests and as expectations in status files. So if a test changed from a runtime error to a compile-time error, we did not notice this at all!
Futhermore: If the dart2js team wanted to fix unexpected compile-time errors first, they had no chance of knowing which tests were failing because of unexpected compile-time errors.

With these two classes we can see the difference, if we put the subclass into the status file as expectation (e.g. status file expectation is RuntimError => we'll detect now if we cannot compile it anymore).

The reason we have subclasses is, that we still have old status files with "Fail" markers. The behavior was to catch runtime and compile-time errors with that marker, to maintain that functionality, we made these two subclasses of fail.

FAIL - test program ran, the assertions that it checked were not satisfied
RUNTIME ERROR - the program threw an unhandled exception and terminated

We sometimes cannot distinguish between unhandled exceptions and failed assertion, because failed assertions will throw an uncaught exception => they result in the same uncaught exception.

About the output readability: Peter filed a bug about this. Maybe @­ricow/@whesse can take a look at this. It's basically interpreting '\n' of the value in the json messages.