Changed: TestCase.output -> TestCase.commandOutputs[]

Changed: TestCase.output -> TestCase.commandOutputs[]

Currently the TestCase has exactly one TestOutput. This worked
until now, since we were only interested in the output of the
last command.

This change introduces one CommandOutput for each command which
has been run. Doing so enables us to fetch output (stdin,stderr)
information as well as exitCode ... for all commands.

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

[ricow1, 2012-11-14 16:59:53]

https://codereview.chromium.org/11293278/diff/1/tests/standalone/io/test_runn...
File tests/standalone/io/test_runner_exit_code_script.dart (right):

https://codereview.chromium.org/11293278/diff/1/tests/standalone/io/test_runn...
tests/standalone/io/test_runner_exit_code_script.dart:31: new
Date.now().difference(startTime));
indentation

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:75: Map<Command,CommandOutput>
commandOutputs = new Map<Command,CommandOutput>();
space after ,

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:149: //       commandOutputs contains only
outputs for cmd1 and cmd2.
you could just do
return commandOutputs[commands[commandOutputs.length - 1]];
(check that it is not empty first)

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:151: while (i>=0) {
spaces around >=

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:270: command, exitCode, incomplete,
timedOut, stdout, stderr, time);
one argument per line

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:327: Command command,
indentation

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:338: Command command,
indentation

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:347: incomplete, timedOut, stdout, stderr,
time);
all or one per line

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:349: return new
AnalysisCommandOutputImpl(testCase, command, exitCode,
all or one per line

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:352: return new CommandOutputImpl(testCase,
command, exitCode, incomplete,
all or one per line

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:395: timedOut, stdout, stderr, time) :
indentation

[Bill Hesse, 2012-11-14 17:07:34]

LGTM.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:347: incomplete, timedOut, stdout, stderr,
time);
These arguments should be all on one line or one on one line.  Sorry.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:628: * Called when all commands are
executed. [exitCode] is 0 if all command
Is this comment still correct?  Do we reach here after only some commands are
executed, if one fails?

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:633: var lastCmdOut = new
CommandOutput.fromCase(testCase, lastCommand,
lastCommandOutput, not lastCmdOut.  Never abbreviate words, except Impl.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:670: void commandComplete(Command cmd, int
exitCode) {
command

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:727: void runCommand(Command command, void
cmdCompleteHandler(Command cmd,
Can we leave the parameter names out of the argument types on the handler?

I would rather break the line at the first comma.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:727: void runCommand(Command command, void
cmdCompleteHandler(Command cmd,
commandCompleteHandler

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_suite...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:1508: // When running on a built bot, the
file can be made visible in the
build bot.

[kustermann, 2012-11-14 17:41:22]

PTAL

https://codereview.chromium.org/11293278/diff/1/tests/standalone/io/test_runn...
File tests/standalone/io/test_runner_exit_code_script.dart (right):

https://codereview.chromium.org/11293278/diff/1/tests/standalone/io/test_runn...
tests/standalone/io/test_runner_exit_code_script.dart:31: new
Date.now().difference(startTime));
On 2012/11/14 16:59:53, ricow1 wrote:
> indentation

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:75: Map<Command,CommandOutput>
commandOutputs = new Map<Command,CommandOutput>();
On 2012/11/14 16:59:53, ricow1 wrote:
> space after ,

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:149: //       commandOutputs contains only
outputs for cmd1 and cmd2.
On 2012/11/14 16:59:53, ricow1 wrote:
> you could just do
> return commandOutputs[commands[commandOutputs.length - 1]];
> (check that it is not empty first)

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:151: while (i>=0) {
This code is now removed!
On 2012/11/14 16:59:53, ricow1 wrote:
> spaces around >=

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:270: command, exitCode, incomplete,
timedOut, stdout, stderr, time);
On 2012/11/14 16:59:53, ricow1 wrote:
> one argument per line

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:327: Command command,
On 2012/11/14 16:59:53, ricow1 wrote:
> indentation

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:338: Command command,
On 2012/11/14 16:59:53, ricow1 wrote:
> indentation

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:347: incomplete, timedOut, stdout, stderr,
time);
On 2012/11/14 16:59:53, ricow1 wrote:
> all or one per line

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:347: incomplete, timedOut, stdout, stderr,
time);
On 2012/11/14 17:07:34, Bill Hesse wrote:
> These arguments should be all on one line or one on one line.  Sorry.

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:349: return new
AnalysisCommandOutputImpl(testCase, command, exitCode,
On 2012/11/14 16:59:53, ricow1 wrote:
> all or one per line

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:352: return new CommandOutputImpl(testCase,
command, exitCode, incomplete,
On 2012/11/14 16:59:53, ricow1 wrote:
> all or one per line

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:395: timedOut, stdout, stderr, time) :
On 2012/11/14 16:59:53, ricow1 wrote:
> indentation

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:628: * Called when all commands are
executed. [exitCode] is 0 if all command
I think so. In the commandComplete function below, testComplete is called if
"exitCode != 0".
On 2012/11/14 17:07:34, Bill Hesse wrote:
> Is this comment still correct?  Do we reach here after only some commands are
> executed, if one fails?

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:633: var lastCmdOut = new
CommandOutput.fromCase(testCase, lastCommand,
On 2012/11/14 17:07:34, Bill Hesse wrote:
> lastCommandOutput, not lastCmdOut.  Never abbreviate words, except Impl.

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:670: void commandComplete(Command cmd, int
exitCode) {
On 2012/11/14 17:07:34, Bill Hesse wrote:
> command

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:727: void runCommand(Command command, void
cmdCompleteHandler(Command cmd,
On 2012/11/14 17:07:34, Bill Hesse wrote:
> Can we leave the parameter names out of the argument types on the handler?
> 
> I would rather break the line at the first comma.

Done.

https://codereview.chromium.org/11293278/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:727: void runCommand(Command command, void
cmdCompleteHandler(Command cmd,
On 2012/11/14 17:07:34, Bill Hesse wrote:
> commandCompleteHandler

Done.

[ricow1, 2012-11-15 08:08:41]

LGTM with comments updated

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:59: * The output information is stored in a
[TestOutput] instance contained
update comment - we don't have TestOutput anymore. Also, you  should update the
comment to explicitly state the new structure

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:69: * A list of commands to execute. Most
test cases have a single command. Frog
Frog -> Dart2js

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:149: throw new Exception("CommandOutputs is
empty, maybe no command was run?");
I would like to add a litte more context to the throw here - if we actually ever
hit this at least the displayName and possibly also the configurationString
would be nice to see?

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:249: * TestOutput records the output of a
completed test: the process's exit code,
TestOutput -> CommandOutput

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:325: // get a new TestOutput instance.
update comment

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:639: * timer, and recording the results in a
new [TestOutput] object, which it
Update comment: TestOutput -> CommandOutput

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:669: testCase,
strange indentation

[kustermann, 2012-11-15 09:04:43]

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:59: * The output information is stored in a
[TestOutput] instance contained
On 2012/11/15 08:08:41, ricow1 wrote:
> update comment - we don't have TestOutput anymore. Also, you  should update
the
> comment to explicitly state the new structure

Done.

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:69: * A list of commands to execute. Most
test cases have a single command. Frog
On 2012/11/15 08:08:41, ricow1 wrote:
> Frog -> Dart2js

Done.

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:149: throw new Exception("CommandOutputs is
empty, maybe no command was run?");
On 2012/11/15 08:08:41, ricow1 wrote:
> I would like to add a litte more context to the throw here - if we actually
ever
> hit this at least the displayName and possibly also the configurationString
> would be nice to see?

Done.

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:249: * TestOutput records the output of a
completed test: the process's exit code,
On 2012/11/15 08:08:41, ricow1 wrote:
> TestOutput -> CommandOutput

Done.

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:325: // get a new TestOutput instance.
On 2012/11/15 08:08:41, ricow1 wrote:
> update comment

Done.

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:639: * timer, and recording the results in a
new [TestOutput] object, which it
On 2012/11/15 08:08:41, ricow1 wrote:
> Update comment: TestOutput -> CommandOutput

Done.

https://codereview.chromium.org/11293278/diff/5003/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:669: testCase,
On 2012/11/15 08:08:41, ricow1 wrote:
> strange indentation

Done.