[test-runner] Move test case processing beyond the multi-process boundary.

[test-runner] Move test case processing beyond the multi-process boundary.

This will allow moving the test outcome check beyond the
multi-process boundary in a follow up. It'll allow wrapping
more complex test jobs like predicable mode on the multi-
process side, which will make the code easier to maintain.

BUG=

Committed: https://crrev.com/780077f6d1eb3ea2d2a4b3f74a4834268ca1bcce
Cr-Commit-Position: refs/heads/master@{#32373}

[Michael Achenbach, 2015-11-24 13:24:06]

Patchset #2 (id:20001) has been deleted

[Michael Achenbach, 2015-11-24 13:24:37]

Patchset #1 (id:1) has been deleted

[Michael Achenbach, 2015-11-24 13:37:17]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-24 13:37:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/60001

[commit-bot: I haz the power, 2015-11-24 13:42:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-24 13:42:05]

Dry run: Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/8035)

[Michael Achenbach, 2015-11-24 13:42:07]

Description was changed from

==========
[test] Refactoring - make job processing more object-oriented.

BUG=
==========

to

==========
[test-runner] Move test case processing beyond the multi-process boundary.

This will allow moving the test outcome check beyond the
multi-process boundary in a follow up. It'll allow wrapping
more complex test jobs like predicable mode on the multi-
process side, which will make the code easier to maintain.

BUG=
==========

[Michael Achenbach, 2015-11-24 13:51:46]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-24 13:51:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/80001

[commit-bot: I haz the power, 2015-11-24 14:05:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-24 14:05:29]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg/builds/11117)

[Michael Achenbach, 2015-11-25 08:41:38]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-25 08:41:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/100001

[commit-bot: I haz the power, 2015-11-25 08:50:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-25 08:50:03]

Dry run: Try jobs failed on following builders:
  v8_win_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel/builds/12200)

[Michael Achenbach, 2015-11-25 13:19:50]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-25 13:19:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/120001

[commit-bot: I haz the power, 2015-11-25 13:48:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-25 13:48:01]

Dry run: Try jobs failed on following builders:
  v8_win_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel/builds/12229)

[Michael Achenbach, 2015-11-25 20:20:16]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-25 20:20:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/200001

[commit-bot: I haz the power, 2015-11-25 20:49:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-25 20:49:49]

Dry run: This issue passed the CQ dry run.

[Michael Achenbach, 2015-11-27 09:39:41]

machenbach@chromium.org changed reviewers:
+ jkummerow@chromium.org, tandrii@chromium.org

[Michael Achenbach, 2015-11-27 09:39:41]

PTAL. Tested locally and verified that there is no negative performance impact.

I'd move the outcome verification beyond the process boundary in a separate CL
to not bloat this up...

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/execution.py:83: def GetCommand(test, context):
The next two methods are the old GetCommand and GetJob copied 1:1 with some
variable/reference renaming.

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/execution.py:148: if instr.dep_command is not None:
This part is a 1:1 copy from the old RunTest.

[Michael Achenbach, 2015-11-27 09:42:32]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-27 09:42:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/220001

[commit-bot: I haz the power, 2015-11-27 10:02:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-27 10:02:29]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2015-11-27 10:09:00]

LGTM with minor comments.

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/execution.py:122: """Stores data to be send over the
multi-process boundary.
nit: s/send/sent/

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/execution.py:142: start_time = time.time()
It doesn't matter much, but I'd move this to its original position right before
"if instr.dep_command is not None". There's no reason to include the
SetSuiteObject/_GetInstructions calls in the timed section.

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/testsuite.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/testsuite.py:119: def global_initialization(self):
I'd keep the original LoadTestSuite and __init__ without any modifications, make
this a standalone method "SetupWorkingDirectory()", and call it from
run-tests.py around line 605:

    suite = testsuite.TestSuite.LoadTestSuite(
        os.path.join(BASE_DIR, "test", root))
    if suite:
+     suite.SetupWorkingDirectory()
      suites.append(suite)

[Michael Achenbach, 2015-11-27 10:31:21]

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/testsuite.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/testsuite.py:119: def global_initialization(self):
On 2015/11/27 10:09:00, Jakob wrote:
> I'd keep the original LoadTestSuite and __init__ without any modifications,
make
> this a standalone method "SetupWorkingDirectory()", and call it from
> run-tests.py around line 605:
> 
>     suite = testsuite.TestSuite.LoadTestSuite(
>         os.path.join(BASE_DIR, "test", root))
>     if suite:
> +     suite.SetupWorkingDirectory()
>       suites.append(suite)

Method name ack. Regarding calling: I'll have to add it to three different
places then: run-test, run-deopt and network_execution endpoint. Is that ok? It
would be a bit scattered.

[Jakob Kummerow, 2015-11-27 10:34:41]

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/testsuite.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/testsuite.py:119: def global_initialization(self):
On 2015/11/27 10:31:21, Michael Achenbach wrote:
> Method name ack. Regarding calling: I'll have to add it to three different
> places then: run-test, run-deopt and network_execution endpoint. Is that ok?
It
> would be a bit scattered.

Hm... that's a bit less elegant than I was hoping, but I'd still prefer an
explicit call over an implicit side effect.

[Michael Achenbach, 2015-11-27 10:58:30]

All done.

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/execution.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/execution.py:83: def GetCommand(test, context):
On 2015/11/27 09:39:41, Michael Achenbach wrote:
> The next two methods are the old GetCommand and GetJob copied 1:1 with some
> variable/reference renaming.

Done.

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/execution.py:142: start_time = time.time()
On 2015/11/27 10:09:00, Jakob wrote:
> It doesn't matter much, but I'd move this to its original position right
before
> "if instr.dep_command is not None". There's no reason to include the
> SetSuiteObject/_GetInstructions calls in the timed section.

Done.

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
File tools/testrunner/local/testsuite.py (right):

https://codereview.chromium.org/1469833002/diff/220001/tools/testrunner/local...
tools/testrunner/local/testsuite.py:119: def global_initialization(self):
On 2015/11/27 10:34:41, Jakob wrote:
> On 2015/11/27 10:31:21, Michael Achenbach wrote:
> > Method name ack. Regarding calling: I'll have to add it to three different
> > places then: run-test, run-deopt and network_execution endpoint. Is that ok?
> It
> > would be a bit scattered.
> 
> Hm... that's a bit less elegant than I was hoping, but I'd still prefer an
> explicit call over an implicit side effect.

Done.

[Michael Achenbach, 2015-11-27 10:58:35]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-27 10:58:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/240001

[commit-bot: I haz the power, 2015-11-27 11:26:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-27 11:26:10]

Dry run: This issue passed the CQ dry run.

[Michael Achenbach, 2015-11-27 12:50:05]

The CQ bit was checked by machenbach@chromium.org

[Michael Achenbach, 2015-11-27 12:50:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1469833002/#ps240001
(title: "Review")

[commit-bot: I haz the power, 2015-11-27 12:50:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469833002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469833002/240001

[commit-bot: I haz the power, 2015-11-27 12:51:48]

Description was changed from

==========
[test-runner] Move test case processing beyond the multi-process boundary.

This will allow moving the test outcome check beyond the
multi-process boundary in a follow up. It'll allow wrapping
more complex test jobs like predicable mode on the multi-
process side, which will make the code easier to maintain.

BUG=
==========

to

==========
[test-runner] Move test case processing beyond the multi-process boundary.

This will allow moving the test outcome check beyond the
multi-process boundary in a follow up. It'll allow wrapping
more complex test jobs like predicable mode on the multi-
process side, which will make the code easier to maintain.

BUG=
==========

[commit-bot: I haz the power, 2015-11-27 12:51:48]

Committed patchset #11 (id:240001)

[commit-bot: I haz the power, 2015-11-27 12:52:34]

Description was changed from

==========
[test-runner] Move test case processing beyond the multi-process boundary.

This will allow moving the test outcome check beyond the
multi-process boundary in a follow up. It'll allow wrapping
more complex test jobs like predicable mode on the multi-
process side, which will make the code easier to maintain.

BUG=
==========

to

==========
[test-runner] Move test case processing beyond the multi-process boundary.

This will allow moving the test outcome check beyond the
multi-process boundary in a follow up. It'll allow wrapping
more complex test jobs like predicable mode on the multi-
process side, which will make the code easier to maintain.

BUG=

Committed: https://crrev.com/780077f6d1eb3ea2d2a4b3f74a4834268ca1bcce
Cr-Commit-Position: refs/heads/master@{#32373}
==========

[commit-bot: I haz the power, 2015-11-27 12:52:35]

Patchset 11 (id:??) landed as
https://crrev.com/780077f6d1eb3ea2d2a4b3f74a4834268ca1bcce
Cr-Commit-Position: refs/heads/master@{#32373}