Only run the NaCl tests if the test type is DEFAULT, UNIT, or app.

Only run the NaCl tests if the test type is DEFAULT, UNIT, or app.

Also, removes nacltest argument from mojob.py.

R=jamesr@chromium.org, jamesr

Committed: https://chromium.googlesource.com/external/mojo/+/80b66ed6e0727c0c8f803670a3510885c3d0b231

[rudominer, 2015-05-14 22:23:29]

rudominer@chromium.org changed reviewers:
+ jamesr@chromium.org

[rudominer, 2015-05-14 22:27:52]

As discussed over IM...

[jamesr, 2015-05-14 22:44:06]

jamesr@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[jamesr, 2015-05-14 22:44:07]

lgtm but vtl should take a second look

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
File mojo/tools/get_test_list.py (right):

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
mojo/tools/get_test_list.py:127: AddXvfbEntry("NaCl app tests",
maybe just this should be guarded by the "app" config?

[viettrungluu, 2015-05-14 22:49:38]

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
File mojo/tools/get_test_list.py (right):

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
mojo/tools/get_test_list.py:119: ShouldRunTest(Config.TEST_TYPE_DEFAULT,
Config.TEST_TYPE_UNIT, "app") ):
nit: no space before second )

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
mojo/tools/get_test_list.py:127: AddXvfbEntry("NaCl app tests",
On 2015/05/14 22:44:07, jamesr wrote:
> maybe just this should be guarded by the "app" config?

I agree.

(And probably "NaCl tests" shouldn't be run for "app", I guess.)

[rudominer, 2015-05-14 22:51:14]

On 2015/05/14 22:49:38, viettrungluu wrote:
>
https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
> File mojo/tools/get_test_list.py (right):
> 
>
https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
> mojo/tools/get_test_list.py:119: ShouldRunTest(Config.TEST_TYPE_DEFAULT,
> Config.TEST_TYPE_UNIT, "app") ):
> nit: no space before second )
> 
>
https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
> mojo/tools/get_test_list.py:127: AddXvfbEntry("NaCl app tests",
> On 2015/05/14 22:44:07, jamesr wrote:
> > maybe just this should be guarded by the "app" config?
> 
> I agree.
> 
> (And probably "NaCl tests" shouldn't be run for "app", I guess.)

How about if I re-arrange as I just suggested in an inline comment?

[rudominer, 2015-05-14 22:51:44]

oops my inline comment was never published.

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
File mojo/tools/get_test_list.py (right):

https://codereview.chromium.org/1147443002/diff/40001/mojo/tools/get_test_lis...
mojo/tools/get_test_list.py:127: AddXvfbEntry("NaCl app tests",
On 2015/05/14 22:44:07, jamesr wrote:
> maybe just this should be guarded by the "app" config?

How about if I move the AddEntry("Nacl tests") up into the section above where
we have AddXvbfEntry("Unit tests") (along with the check for Linux and !asan)
and similarly for the NaclAppTests.

[rudominer, 2015-05-14 23:16:15]

ptal

[jamesr, 2015-05-14 23:50:41]

Thanks, this is much cleaner

lgtm

[rudominer, 2015-05-14 23:54:44]

Committed patchset #6 (id:100001) manually as
80b66ed6e0727c0c8f803670a3510885c3d0b231 (presubmit successful).