tools/test.dart: Read test expectations from the status file, and pass them to the test runner.

tools/test.dart: Read test expectations from the status file, and pass them to the test runner.

BUG=
TEST=tools/test.dart

Committed: https://code.google.com/p/dart/source/detail?r=1407

[Mads Ager (google), 2011-11-10 16:31:57]

LGTM

http://codereview.chromium.org/8514003/diff/2001/tests/corelib/test_config.dart
File tests/corelib/test_config.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tests/corelib/test_config.da...
tests/corelib/test_config.dart:23: configuration = {"mode": "debug",
In the next changelist we can replace this with the actual options passed in.
:-)

http://codereview.chromium.org/8514003/diff/2001/tests/standalone/src/TestRun...
File tests/standalone/src/TestRunnerTest.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tests/standalone/src/TestRun...
tests/standalone/src/TestRunnerTest.dart:8:
#import("../../../tools/testing/dart/status_file_parser.dart");
Do you need this here when there are no other changes to this file?

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
File tools/testing/dart/status_file_parser.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:33: condition == null ||
condition.evaluate(environment);
Could we make condition just be "true" or something so we don't have to check
for null? Or is "true" not part of the condition language?

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:128: // Debugging code.
Remove this before committing?

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:142: Collection<String>
ExpectationsFromFile(String fileName) {
Unused I think? Remove?

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/test_runn...
File tools/testing/dart/test_runner.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:79: bool get hasCrashed() => !timedOut &&
exitCode != 255 && exitCode != 0;
This looks accidental?

[Bill Hesse, 2011-11-10 16:44:45]

http://codereview.chromium.org/8514003/diff/2001/tests/standalone/src/TestRun...
File tests/standalone/src/TestRunnerTest.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tests/standalone/src/TestRun...
tests/standalone/src/TestRunnerTest.dart:8:
#import("../../../tools/testing/dart/status_file_parser.dart");
On 2011/11/10 16:31:57, Mads Ager wrote:
> Do you need this here when there are no other changes to this file?

Yes.  The declarations of PASS, FAIL, and CRASH are in status_file_parser.dart. 
We may need a base file containing the core declarations, to avoid circular
includes.

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
File tools/testing/dart/status_file_parser.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:33: condition == null ||
condition.evaluate(environment);
On 2011/11/10 16:31:57, Mads Ager wrote:
> Could we make condition just be "true" or something so we don't have to check
> for null? Or is "true" not part of the condition language?

No, constants or a built-in true are not part of the condition language.  Also,
negation is not, so it cannot be written as !$checked || $checked.

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:128: // Debugging code.
On 2011/11/10 16:31:57, Mads Ager wrote:
> Remove this before committing?

I'll just remove the comment.  The output is quite useful.

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:142: Collection<String>
ExpectationsFromFile(String fileName) {
On 2011/11/10 16:31:57, Mads Ager wrote:
> Unused I think? Remove?

Done.

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/test_runn...
File tools/testing/dart/test_runner.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:79: bool get hasCrashed() => !timedOut &&
exitCode != 255 && exitCode != 0;
On 2011/11/10 16:31:57, Mads Ager wrote:
> This looks accidental?

No, the processes are returning 255 for uncaught exceptions.
So 255 is a fail, not a crash.

[Bill Hesse, 2011-11-14 10:08:04]

Nothing new here - just committing my draft comments on a closed issue.

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
File tools/testing/dart/status_file_parser.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/status_fi...
tools/testing/dart/status_file_parser.dart:128: // Debugging code.
Removed the whole thing.

On 2011/11/10 16:44:45, Bill Hesse wrote:
> On 2011/11/10 16:31:57, Mads Ager wrote:
> > Remove this before committing?
> 
> I'll just remove the comment.  The output is quite useful.

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/test_runn...
File tools/testing/dart/test_runner.dart (right):

http://codereview.chromium.org/8514003/diff/2001/tools/testing/dart/test_runn...
tools/testing/dart/test_runner.dart:79: bool get hasCrashed() => !timedOut &&
exitCode != 255 && exitCode != 0;
OK, this has been fixed, so failures return -1, so will remove.

On 2011/11/10 16:44:45, Bill Hesse wrote:
> On 2011/11/10 16:31:57, Mads Ager wrote:
> > This looks accidental?
> 
> No, the processes are returning 255 for uncaught exceptions.
> So 255 is a fail, not a crash.