642299 - Add jstests to make check, with jitflags

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Paul Biggar]

Attachment: Add jstests to |make check|

jstests are exercised in the browser, through jsreftests. This means they're only tested with the browser build configuration, which is -jmp. MEanwhile, jit-tests are exercised with a much larger variety of flags, currently ,m,j,mj,mjp,am,amj,amjp,amd. We'd like to run jstests.py with the same flags.

The simplest way to do this is to parse JITFLAGS and run jstests.py once for each JITFLAGS. The cleanest way to do it is to merge jstests.py and jit_test.py. The second cleanest way to do it is to add the jit-flag parsing machinery from jit-tests.

This patch opts for the simplest way, in the hope that it'll be replaced in the future when someone merges the two JS test programs.

The output is a bit annoying as it doesn't contain the flags used, but I'm fixing that in bug 642298.

Comment 1

[Paul Biggar]

This brings up some latent failures in tests (see http://tbpl.mozilla.org/?tree=MozillaTry&rev=066ccf1aa833).

32-bit:
TEST-UNEXPECTED-FAIL | -a -m | js1_8_5/extensions/worker-terminate.js | Got error: undefined : Expected value '0', Actual value '1' 
TEST-UNEXPECTED-FAIL | -a -m -d | js1_8/extensions/regress-422269.js

64-bit:
TEST-UNEXPECTED-FAIL | -j | ecma_5/Object/15.2.3.6-dictionary-redefinition-1-of-8.js
TEST-UNEXPECTED-FAIL | -j | ecma_5/Object/15.2.3.6-dictionary-redefinition-2-of-8.js
TEST-UNEXPECTED-FAIL | -j | ecma_5/Object/15.2.3.6-dictionary-redefinition-3-of-8.js
TEST-UNEXPECTED-FAIL | -j | ecma_5/Object/15.2.3.6-dictionary-redefinition-4-of-8.js
TEST-UNEXPECTED-FAIL | -j | ecma_5/Object/15.2.3.6-dictionary-redefinition-5-of-8.js
TEST-UNEXPECTED-FAIL | -j | ecma_5/Object/15.2.3.6-dictionary-redefinition-6-of-8.js
TEST-UNEXPECTED-FAIL | -m -j -p | js1_8_5/extensions/worker-terminate.js | Got error: undefined : Expected value '0', Actual value '1' 
TEST-UNEXPECTED-FAIL | -a -m | js1_5/Regress/regress-416628.js
TEST-UNEXPECTED-FAIL | -a -m -j | js1_5/Regress/regress-416628.js
TEST-UNEXPECTED-FAIL | -a -m -j | js1_8/extensions/regress-476427.js
TEST-UNEXPECTED-FAIL | -a -m -j | js1_8_5/extensions/worker-terminate.js | Got error: undefined : Expected value '0', Actual value '1' 
TEST-UNEXPECTED-FAIL | -a -m -j -p | js1_5/Regress/regress-416628.js
TEST-UNEXPECTED-FAIL | -a -m -d | js1_5/Regress/regress-416628.js
TEST-UNEXPECTED-FAIL | -a -m -d | js1_8_5/extensions/worker-terminate.js | Got error: undefined : Expected value '0', Actual value '1'

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Most of these are probably timeouts. They're all really long-running tests, except for the worker one. You could try passing in a larger timeout, with -t.

I tried to reproduce the worker one locally, but I couldn't. Have you been able to get it?

Comment 3

[Paul Biggar]

Yup, I've confirmed their all timeouts except the worker one, which I also can't reproduce.

Anyone have an opinion on whether I just should set the timeout higher? It's already 150 seconds I think.

Comment 4

[David Mandelin [:dmandelin]]

(In reply to comment #3)
> Yup, I've confirmed their all timeouts except the worker one, which I also
> can't reproduce.
> 
> Anyone have an opinion on whether I just should set the timeout higher? It's
> already 150 seconds I think.

I don't think we should be running very many super-long-running tests for every checkin. Maybe we should just disable them for tinderbox runs or something.

Comment 5

[Paul Biggar]

(In reply to comment #4)
> I don't think we should be running very many super-long-running tests for every
> checkin. Maybe we should just disable them for tinderbox runs or something.

Is there a way to do that in the manifest?

Comment 6

[David Mandelin [:dmandelin]]

(In reply to comment #5)
> (In reply to comment #4)
> > I don't think we should be running very many super-long-running tests for every
> > checkin. Maybe we should just disable them for tinderbox runs or something.
> 
> Is there a way to do that in the manifest?

I don't think there is currently.

Comment 7

[Paul Biggar]

(In reply to comment #4)
> > Anyone have an opinion on whether I just should set the timeout higher? It's
> > already 150 seconds I think.
> 
> I don't think we should be running very many super-long-running tests for every
> checkin. Maybe we should just disable them for tinderbox runs or something.

I had a closer look and the tests aren't super-long-running, they take about 14 seconds each on my machine (which is about what the longest jit-tests take too). The releng machines are pretty slow, which is why they took so long. So I think the best thing to do is turn up the timeout.

Alternatively, I can mark them as slow. That will mean they aren't run as part of a developer's jstests, and aren't run on tbpl (which is already the case).

Comment 8

[Jeff Walden [:Waldo]]

The browser-based JS tests run the slow tests.  It's only if you're running it in the shell, where you can get parallelism and other stuff, that they're not run.

Comment 9

[Paul Biggar]

The tests are currently marked as shell-only, which is why the browser doesn't run them. If we marked them as slow, jstests.py wouldn't run them either.

As such, I much prefer just setting the timeout higher.

Comment 10

[Paul Biggar]

Attachment: Add js_tests with a high timeout and no extensions tests

This also prints "TIMEOUT" to tinderbox output when we timeout.

Comment 11

[(not currently active) Ted Mielczarek]

I'm not sure why we didn't run these in the first place, except maybe duplication since they're already running in the browser?

Comment 12

[Bob Clary [:bc] (inactive)]

They aren't really a duplicate since some tests require features only available in the shell. This will be good added coverage.

Comment 13

[Nicholas Nethercote [inactive]]

(In reply to comment #7)
> 
> Alternatively, I can mark them as slow. That will mean they aren't run as part
> of a developer's jstests

I'd love that.  jstests.py always has a huge pause in the middle which is really annoying.  I've been wanting to mark them as slow for ages, just haven't got around to it.  If they were somehow run on TB but not locally that would be ideal -- we'd still get the coverage.

Comment 14

[Paul Biggar]

http://hg.mozilla.org/tracemonkey/rev/8e91cb98a083
http://hg.mozilla.org/tracemonkey/rev/508ced87a3f8

Comment 15

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/8e91cb98a083
http://hg.mozilla.org/mozilla-central/rev/508ced87a3f8