Fuchsia: Improves run_vm_tests wrapper program.

Fuchsia: Improves run_vm_tests wrapper program.

This change makes a few improvements to Fuchsia's wrapper around
run_vm_tests:
- Loads the binary into memory only once, speeding things up a bit.
- Captures the output of 'run_vm_tests --list' to extract the list
  of tests to run instead of reading it from a file.
- Captures the output of the tests, and prints a test's stdout
  and stderr to the console only if the test failed.

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/c21891ec6a143c1188581b54d26c3ab92ae47ec9

[zra, 2016-08-11 21:13:15]

zra@google.com changed reviewers:
+ asiva@google.com

[zra, 2016-08-17 21:54:06]

ping

[siva, 2016-08-18 00:38:46]

lgtm

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
File runtime/bin/run_vm_tests_fuchsia.cc (right):

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:169: launchpad_destroy(lp);
Missing
if (stdout_out != NULL) {
  close(*stdout_out);
}

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:181: }
ASSERT(lp_out != NULL);

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:246: // size = size * 1.5.
maybe // new_size = size * 1.5.

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:344: int test_status = run_test(vmo_dup,
test, &test_stdout, &test_stderr);
should this be :
if (test_status != -1) {
  handle_result(...);
  free(test_stdout);
  free(test_stderr);
}

[zra, 2016-08-18 15:49:32]

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
File runtime/bin/run_vm_tests_fuchsia.cc (right):

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:169: launchpad_destroy(lp);
On 2016/08/18 00:38:46, siva wrote:
> Missing
> if (stdout_out != NULL) {
>   close(*stdout_out);
> }

Moved error handling to the caller.

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:181: }
On 2016/08/18 00:38:46, siva wrote:
> ASSERT(lp_out != NULL);

Done.

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:246: // size = size * 1.5.
On 2016/08/18 00:38:46, siva wrote:
> maybe // new_size = size * 1.5.

Done.

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:344: int test_status = run_test(vmo_dup,
test, &test_stdout, &test_stderr);
On 2016/08/18 00:38:46, siva wrote:
> should this be :
> if (test_status != -1) {
>   handle_result(...);
>   free(test_stdout);
>   free(test_stderr);
> }

-1 is a return code from run_test that we need to report/handle. free() is NULL
safe.

[zra, 2016-08-18 15:50:30]

Description was changed from

==========
Fuchsia: Improves run_vm_tests wrapper program.

This change makes a few improvements to Fuchsia's wrapper around
run_vm_tests:
- Loads the binary into memory only once, speeding things up a bit.
- Captures the output of 'run_vm_tests --list' to extract the list
  of tests to run instead of reading it from a file.
- Captures the output of the tests, and prints a test's stdout
  and stderr to the console only if the test failed.
==========

to

==========
Fuchsia: Improves run_vm_tests wrapper program.

This change makes a few improvements to Fuchsia's wrapper around
run_vm_tests:
- Loads the binary into memory only once, speeding things up a bit.
- Captures the output of 'run_vm_tests --list' to extract the list
  of tests to run instead of reading it from a file.
- Captures the output of the tests, and prints a test's stdout
  and stderr to the console only if the test failed.

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c21891ec6a143c1188581b54d26c3ab92ae47ec9
==========

[zra, 2016-08-18 15:50:31]

Committed patchset #2 (id:20001) manually as
c21891ec6a143c1188581b54d26c3ab92ae47ec9 (presubmit successful).

[siva, 2016-08-18 16:06:12]

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
File runtime/bin/run_vm_tests_fuchsia.cc (right):

https://codereview.chromium.org/2238983003/diff/1/runtime/bin/run_vm_tests_fu...
runtime/bin/run_vm_tests_fuchsia.cc:344: int test_status = run_test(vmo_dup,
test, &test_stdout, &test_stderr);
On 2016/08/18 15:49:31, zra wrote:
> On 2016/08/18 00:38:46, siva wrote:
> > should this be :
> > if (test_status != -1) {
> >   handle_result(...);
> >   free(test_stdout);
> >   free(test_stderr);
> > }
> 
> -1 is a return code from run_test that we need to report/handle. free() is
NULL
> safe.

but test_stdout and test_stderr are not being set to NULL after the free when
return code is -1.