735549 - Generate the jsreftest manifests at build time

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

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 jstests.py.

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

[Bob Clary [:bc] (inactive)]

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?

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

I don't understand the entire flow here, but let me outline what we do for mobile, 

* We download the tests.zip file and unpack it
* we setup a webserver with the root at the tests.zip:/reftest 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

[Terrence Cole [:terrence]]

Good!  That's basically how I thought the workflow went.  My plan is to end up giving you a tests.zip 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

[Terrence Cole [:terrence]]

Attachment: 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

[Terrence Cole [:terrence]]

Attachment: wip: still failing on try

Comment 6

[Terrence Cole [:terrence]]

Attachment: v1: Added url-prefix and removed dups

Still failing, but only on windows now.  This is getting close.

Comment 7

[Terrence Cole [:terrence]]

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

[Terrence Cole [:terrence]]

Attachment: v2: Had a '/' where I should have had an os.sep.

https://tbpl.mozilla.org/?tree=Try&rev=cb6d4d86df3e

542 files changed, 633 insertions(+), 3922 deletions(-)

Comment 9

[David Mandelin [: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).

Comment 10

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/61189954ca17

Comment 11

[Bob Clary [:bc] (inactive)]

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

Comment 12

[Terrence Cole [:terrence]]

(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.

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/61189954ca17

Comment 14

[Bob Clary [:bc] (inactive)]

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

[Terrence Cole [:terrence]]

Well, this bug got marked fixed, so we better open a new one.  Bug 746236