Further deduplicate and simplify PNaCl SCons testing

Further deduplicate and simplify PNaCl SCons testing

Remove more duplication between buildbot_pnacl.py and buildbot_pnacl.sh:
1) Make the x86-64 toolchain bot use buildbot_pnacl.py to run SCons tests.
2) Update the non-SFI tests in buildbot_pnacl.py to reflect the current state
on ARM and x86 and update the ARM QEMU trybot to use buildbot_pnacl.py
to run SCons tests. Note that the nacl-arm_opt_panda trybot and the buildbot
(which both use hardware) are not affected by this change.
Only the nacl-precise_64-newlib-arm_qemu-pnacl trybot is affected.

A future CL will move the scons testing logic for the ARM hardware bots, and
then we can remove all the nonsfi stuff from buildbot_pnacl.sh.

R=mseaborn@chromium.org, hidehiko@chromium.org, jvoung@chromium.org
BUG= http://code.google.com/p/chromium/issues/detail?id=358465

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=13581

[Mark Seaborn, 2014-07-28 21:12:32]

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (left):

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl....
buildbot/buildbot_pnacl.py:20: RemoveGypBuildDirectories()
What's the reason for removing this?  It doesn't seem to be moved elsewhere.

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl.sh
File buildbot/buildbot_pnacl.sh (left):

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl....
buildbot/buildbot_pnacl.sh:65: readonly SCONS_NONSFI_TC="\
This overlaps with Hidehiko's change
(https://codereview.chromium.org/415383002/).  Since it's smaller in scope, I'd
rather like to see it committed first...  Plus it has trybot runs ready whereas
this doesn't. :-)

[Derek Schuff, 2014-07-28 21:16:06]

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (left):

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl....
buildbot/buildbot_pnacl.py:20: RemoveGypBuildDirectories()
On 2014/07/28 21:12:31, Mark Seaborn wrote:
> What's the reason for removing this?  It doesn't seem to be moved elsewhere.

There's no gyp build anywhere in this script.

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl.sh
File buildbot/buildbot_pnacl.sh (left):

https://codereview.chromium.org/426763002/diff/20001/buildbot/buildbot_pnacl....
buildbot/buildbot_pnacl.sh:65: readonly SCONS_NONSFI_TC="\
On 2014/07/28 21:12:31, Mark Seaborn wrote:
> This overlaps with Hidehiko's change
> (https://codereview.chromium.org/415383002/).  Since it's smaller in scope,
I'd
> rather like to see it committed first...  Plus it has trybot runs ready
whereas
> this doesn't. :-)

Acknowledged.

[Derek Schuff, 2014-07-28 22:35:01]

rebased onto HEAD after hidehido's change landed.
The one difference I can see is that the buildbot_pnacl.py list now runs
stack_alignment_irt with host libc whereas buildbot_pnacl.sh does not. Was that
intentional?

[Mark Seaborn, 2014-07-29 17:11:33]

I looked at the trybot output:
http://build.chromium.org/p/tryserver.nacl/builders/nacl-toolchain-linux-pnac...

This now shows steps like "large_tests".  The arch information is gone from the
step name.  Can you add that to buildbot_pnacl.py?  I think it would be OK to
add the arch unconditionally.

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:60: # The ARM sandboxed translator is flaky under
qemu.
Maybe add "...so run a very small set of tests there"

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl.sh
File buildbot/buildbot_pnacl.sh (right):

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.sh:352: scons-stage-irt "arm" "${qemuflags}"
"${SCONS_NONSFI_NEWLIB}"
Can you convert this (mode-buildbot-arm*) too?

Otherwise the ARM trybot and buildbot will be diverging further by being defined
in different files, with different test lists.  There would be a bigger chance
that a change will pass on the trybots but fail on the main bots.

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.sh:450: ./scons --verbose platform=x86-32 -j8 sel_ldr
I'm curious: How come this isn't covered by BUILDBOT_PNACL_PY earlier?  Does
that build the dbg version?

[Derek Schuff, 2014-07-29 18:41:50]

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:60: # The ARM sandboxed translator is flaky under
qemu.
On 2014/07/29 17:11:32, Mark Seaborn wrote:
> Maybe add "...so run a very small set of tests there"

Done.

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl.sh
File buildbot/buildbot_pnacl.sh (right):

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.sh:352: scons-stage-irt "arm" "${qemuflags}"
"${SCONS_NONSFI_NEWLIB}"
On 2014/07/29 17:11:33, Mark Seaborn wrote:
> Can you convert this (mode-buildbot-arm*) too?
> 
> Otherwise the ARM trybot and buildbot will be diverging further by being
defined
> in different files, with different test lists.  There would be a bigger chance
> that a change will pass on the trybots but fail on the main bots.

The ARM buildbot still matches the nacl-arm_opt_panda trybot, and uses this
code. The change in this cl only affects the
nacl-precise_64-newlib-arm_qemu-pnacl trybot, which uses qemu rather than
hardware. I updated the CL description to make this more clear.

https://codereview.chromium.org/426763002/diff/120001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.sh:450: ./scons --verbose platform=x86-32 -j8 sel_ldr
On 2014/07/29 17:11:33, Mark Seaborn wrote:
> I'm curious: How come this isn't covered by BUILDBOT_PNACL_PY earlier?  Does
> that build the dbg version?

It's because the python version clobbers all the scons output directories (for
all architectures) when it starts. So when the 64 bit tests run it clobbers the
32 bit sel_ldr that was there.

[Derek Schuff, 2014-07-30 17:59:39]

i think it's ready now. any takers?

[Mark Seaborn, 2014-07-31 15:52:03]

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:72: with Step('sandboxed_translator_fast_tests' +
arch, status,
Missing space

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:81: # versions of are currently x86-32 and ARM, and
ARM testing is covered
Need to remove "ARM testing is covered buildbot_pnacl.sh rather than this Python
script".

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:88: nonsfi_tests_common  = ['run_' + test +
'_test_irt' for test in
I found the Non-SFI part of the refactoring really difficult to review.  It's
hard for me to compare the lists of tests.

I'm going to send you a cleanup change that makes this a lot simpler:
https://codereview.chromium.org/424763007/

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl.sh
File buildbot/buildbot_pnacl.sh (right):

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.sh:388: ${BUILDBOT_PNACL_PY} opt arm pnacl
Suggestion:  Could you just write buildbot/buildbot_pnacl.py here?  Indirecting
this via a variable makes the code harder to read, IMHO.

[Derek Schuff, 2014-07-31 16:18:09]

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:72: with Step('sandboxed_translator_fast_tests' +
arch, status,
On 2014/07/31 15:52:03, Mark Seaborn wrote:
> Missing space

Done.

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:81: # versions of are currently x86-32 and ARM, and
ARM testing is covered
On 2014/07/31 15:52:03, Mark Seaborn wrote:
> Need to remove "ARM testing is covered buildbot_pnacl.sh rather than this
Python
> script".

Done.

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:88: nonsfi_tests_common  = ['run_' + test +
'_test_irt' for test in
On 2014/07/31 15:52:03, Mark Seaborn wrote:
> I found the Non-SFI part of the refactoring really difficult to review.  It's
> hard for me to compare the lists of tests.
> 
> I'm going to send you a cleanup change that makes this a lot simpler:
> https://codereview.chromium.org/424763007/

looks good.

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl.sh
File buildbot/buildbot_pnacl.sh (right):

https://codereview.chromium.org/426763002/diff/180001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.sh:388: ${BUILDBOT_PNACL_PY} opt arm pnacl
On 2014/07/31 15:52:03, Mark Seaborn wrote:
> Suggestion:  Could you just write buildbot/buildbot_pnacl.py here? 
Indirecting
> this via a variable makes the code harder to read, IMHO.

I actually agree, although all of our shell scripts seem to have always used the
variables. I changed it here.

[Derek Schuff, 2014-07-31 18:12:01]

rebased again, PTAL.

[Mark Seaborn, 2014-08-04 20:39:57]

LGTM

I checked this by making a list of the test suites that the scripts run before,
and checked that they run them after too:

mode-trybot-qemu:

 * scons, build-only
 * small+medium tests
 * large tests
 * small+medium IRT tests
 * nonpexe_tests
 * nonsfi tests (newlib + host)

tests-tc-all:

 * for each arch:
     SCONS_TC_TESTS (small/medium)
     large_tests
     nonpexe_tests
 * sandboxed translator:  toolchain_tests for arches * (O0,O2)
 * SCONS_NONSFI * 2 arches
 * unsandboxed hello_world for x86-32

https://codereview.chromium.org/426763002/diff/200001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/426763002/diff/200001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:84: with Step('nonsfi_tests', status,
halt_on_fail=False):
Can you add "+ arch" here?

https://codereview.chromium.org/426763002/diff/200001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:92: with Step('nonsfi_tests_host_libc', status,
halt_on_fail=False):
Can you add "+ arch" here?

[Derek Schuff, 2014-08-04 21:16:38]

https://codereview.chromium.org/426763002/diff/200001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/426763002/diff/200001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:84: with Step('nonsfi_tests', status,
halt_on_fail=False):
On 2014/08/04 20:39:57, Mark Seaborn wrote:
> Can you add "+ arch" here?

Done.

https://codereview.chromium.org/426763002/diff/200001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:92: with Step('nonsfi_tests_host_libc', status,
halt_on_fail=False):
On 2014/08/04 20:39:57, Mark Seaborn wrote:
> Can you add "+ arch" here?

Done.

[Derek Schuff, 2014-08-04 21:17:22]

Committed patchset #12 manually as r13581 (presubmit successful).