Closed Bug 918934 Opened 6 years ago Closed 6 years ago

Update jit-tests remote harness

Categories

(Core :: JavaScript Engine, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(1 file)

I need to make some changes to how the jit-tests are packaged so that relative paths in the JavaScript include paths work as expected. This in turn requires some small changes to paths in the remote test harness.
No longer depends on: 916742
Try run here: https://tbpl.mozilla.org/?tree=Try&rev=1f58ae747f29

I've added the ecma_6 directory and I've changed the directory structure on the remote device. We used to flatten the hierarchy in a way that breaks relative paths, so I've made things match the local hierarchy.
Attachment #808526 - Flags: review?(terrence)
Comment on attachment 808526 [details] [diff] [review]
Patch to add ecma_6 and change directory structure on remote device.

Review of attachment 808526 [details] [diff] [review]:
-----------------------------------------------------------------

The path changes look good. We probably should have set it up like that in the first place. One question however:

::: js/src/tests/lib/jittests.py
@@ +27,5 @@
>  JS_DIR = os.path.dirname(os.path.dirname(TESTS_LIB_DIR))
>  TOP_SRC_DIR = os.path.dirname(os.path.dirname(JS_DIR))
>  TEST_DIR = os.path.join(JS_DIR, 'jit-test', 'tests')
>  LIB_DIR = os.path.join(JS_DIR, 'jit-test', 'lib') + os.path.sep
> +ECMA6_DIR = posixpath.join(JS_DIR, 'tests', 'ecma_6')

Can you explain why jittests needs to know about this directory at all? This would seem to be a random directory for a different test harness and jittests really should not be depending on it.
Comment on attachment 808526 [details] [diff] [review]
Patch to add ecma_6 and change directory structure on remote device.

Review of attachment 808526 [details] [diff] [review]:
-----------------------------------------------------------------

The path changes look good. We probably should have set it up like that in the first place. One question however:

::: js/src/tests/lib/jittests.py
@@ +27,5 @@
>  JS_DIR = os.path.dirname(os.path.dirname(TESTS_LIB_DIR))
>  TOP_SRC_DIR = os.path.dirname(os.path.dirname(JS_DIR))
>  TEST_DIR = os.path.join(JS_DIR, 'jit-test', 'tests')
>  LIB_DIR = os.path.join(JS_DIR, 'jit-test', 'lib') + os.path.sep
> +ECMA6_DIR = posixpath.join(JS_DIR, 'tests', 'ecma_6')

Can you explain why jittests needs to know about this directory at all? This would seem to be a random directory for a different test harness and jittests really should not be depending on it.
Attachment #808526 - Flags: review?(terrence) → review+
After discussion on IRC, #jsapi decided this is gross, but better than duplicating the shared code.
Thanks for the review.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d7b910c458af
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/d7b910c458af
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 858621
You need to log in before you can comment on or make changes to this bug.