Generate the jsreftest manifests at build time

RESOLVED FIXED in mozilla14



JavaScript Engine
6 years ago
5 years ago


(Reporter: terrence, Assigned: terrence)



Firefox Tracking Flags

(Not tracked)



(2 attachments, 2 obsolete attachments)



6 years ago
The goal of this bug is to get rid of the need to update a manifest file when adding a test or test directory to the jsreftest suite.

The best way to do this is to store the test metadata inline in the js test files, as is done by the jit tests.  With a fairly small amount of code, we should be able to dump out the reftest manifests what would be required to get the same result on command, say by passing -M or -R or something to

We can then add this command to the build step that generates tarballs for the reftest runner that is used by tbpl and never have to worry about manifest files ever again.

Comment 1

6 years ago
terrence: This sounds like a good plan. I just noticed the jit-test marking today when looking at a patch dmandelin landed. Let me know where I can help.

jmaher: Just confirming this won't affect mobile testing, right?
I don't understand the entire flow here, but let me outline what we do for mobile, 

* We download the file and unpack it
* we setup a webserver with the root at the directory
* we launch fennec on the remote device with a reference to http://<ip>/tests/layout/reftests/reftest.list (different url for jsreftests and crashtests)
* this expects the reftest harness to parse the list and run the tests

Comment 3

6 years ago
Good!  That's basically how I thought the workflow went.  My plan is to end up giving you a with a functionally identical reftest.list.  Ideally, you should not have to even know that that zip is being generated in a different way.

Comment 4

6 years ago
Created attachment 605998 [details] [diff] [review]
A script to update all required tests.

Of the ~4000 tests we run, 377 require special treatment inside the manifest.  This script writes the information in those 377 lines directly into the affected tests inside of a special comment on the first line.

Comment 5

5 years ago
Created attachment 612924 [details] [diff] [review]
wip: still failing on try

Comment 6

5 years ago
Created attachment 614461 [details] [diff] [review]
v1: Added url-prefix and removed dups

Still failing, but only on windows now.  This is getting close.
Attachment #612924 - Attachment is obsolete: true

Comment 7

5 years ago
In windows the test page we load is jsreftest.html?test=/foo/bar/baz.js and everywhere else it is jsreftest.html?test=foo/bar/baz.js (as intended).  I expect this is down to a platform difference in os.path -- need to investigate what still, but it should be easy to track down.

Comment 8

5 years ago
Created attachment 614654 [details] [diff] [review]
v2: Had a '/' where I should have had an os.sep.

542 files changed, 633 insertions(+), 3922 deletions(-)
Attachment #614461 - Attachment is obsolete: true
Attachment #614654 - Flags: review?(dmandelin)
Comment on attachment 614654 [details] [diff] [review]
v2: Had a '/' where I should have had an os.sep.

Review of attachment 614654 [details] [diff] [review]:

Please add a note to the README about the metadata format, I think just a link to the reftest docs (don't want redundant docs).
Attachment #614654 - Flags: review?(dmandelin) → review+

Comment 10

5 years ago

Comment 11

5 years ago
Doesn't this break the ability to do a  make -C firefox-debug jstestbrowser to run the jsreftests locally?

Comment 12

5 years ago
(In reply to Bob Clary [:bc:] from comment #11)
> Doesn't this break the ability to do a  make -C firefox-debug jstestbrowser
> to run the jsreftests locally?

I didn't even know you could do that.  I'll take a look at it.
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14

Comment 14

5 years ago
Terrence, would you like me to file a new bug or would you like to take care of the make -C firefox-debug jstestbrowser issue here?

Comment 15

5 years ago
Well, this bug got marked fixed, so we better open a new one.  Bug 746236
Depends on: 779688
You need to log in before you can comment on or make changes to this bug.