Add jstests to make check, with jitflags

RESOLVED FIXED in mozilla2.0

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Paul Biggar, Assigned: Paul Biggar)

Tracking

Trunk
mozilla2.0
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-tracemonkey])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Created attachment 519794 [details] [diff] [review]
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.
(Assignee)

Comment 1

7 years ago
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'
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?
(Assignee)

Comment 3

7 years ago
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.
(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.
(Assignee)

Comment 5

7 years ago
(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?
(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.
(Assignee)

Updated

7 years ago
Depends on: 644393
(Assignee)

Comment 7

7 years ago
(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).
Target Milestone: --- → mozilla2.0

Comment 8

7 years ago
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.
(Assignee)

Comment 9

7 years ago
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.
(Assignee)

Comment 10

7 years ago
Created attachment 521707 [details] [diff] [review]
Add js_tests with a high timeout and no extensions tests

This also prints "TIMEOUT" to tinderbox output when we timeout.
Assignee: general → pbiggar
Attachment #519794 - Attachment is obsolete: true
Attachment #521707 - Flags: review?(dmandelin)
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?
They aren't really a duplicate since some tests require features only available in the shell. This will be good added coverage.
(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.
Attachment #521707 - Flags: review?(dmandelin) → review+
You need to log in before you can comment on or make changes to this bug.