1426006 - (--disable-ion) testPreserveJitCode.cpp:78:CHECK_EQUAL failed: expected (1u) = 1, got (countIonScripts(global)) = 0 | testPreserveJitCode.cpp:22:CHECK failed: testPreserveJitCode(false, 0)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Mike Hommey [:glandium]]

Bug 1426003 adds build jobs with --disable-ion. The tests are disabled in those jobs because there are various failures.

This bug is about the following jsapi-tests failure:
[task 2017-12-18T23:48:47.853Z] test_PreserveJitCode
[task 2017-12-18T23:48:47.853Z] TEST-UNEXPECTED-FAIL | test_PreserveJitCode | /builds/worker/workspace/build/src/js/src/jsapi-tests/testPreserveJitCode.cpp:78:CHECK_EQUAL failed: expected (1u) = 1, got (countIonScripts(global)) = 0 | /builds/worker/workspace/build/src/js/src/jsapi-tests/testPreserveJitCode.cpp:22:CHECK failed: testPreserveJitCode(false, 0)

Comment 1

[Jason Orendorff [:jorendorff]]

Steve, could you patch this?

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Do not expect ion scripts when ion is unavailable

You win the random review request recipient contest!

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8953344 [details] [diff] [review]
Do not expect ion scripts when ion is unavailable

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

And you win a non-random r+! Thanks.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a9f2cf4ca3f
Do not expect ion scripts when ion is unavailable, r=bbouvier

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7a9f2cf4ca3f

Comment 6

[Liz Henry (:lizzard)]

Want to request uplift for 59 beta? This could still make Thursday's beta 14 build.

Comment 7

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8953344 [details] [diff] [review]
Do not expect ion scripts when ion is unavailable

Approval Request Comment
[Feature/Bug causing the regression]: bitrot of untested configuration
[User impact if declined]: none
[Is this code covered by automated tests?]: yes, bug 1426003, but it's disabled while it's broken
[Has the fix been verified in Nightly?]: not used in Nightly, so no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: bug 1426008, which I haven't gotten to yet
[Is the change risky?]: no
[Why is the change risky/not risky?]: not part of any shipping build
[String changes made/needed]: none

To be clear, I would like to uplift this both for tier 3 platforms that use Spidermonkey without the JIT, and for differential testing. Uplifting this bug won't fix the problem, but it's safe to uplift this independently.

Comment 8

[Liz Henry (:lizzard)]

Comment on attachment 8953344 [details] [diff] [review]
Do not expect ion scripts when ion is unavailable

Let's uplift for beta 14.

Comment 9

[Liz Henry (:lizzard)]

I didn't notice that this needs bug 1426008.  Looks like we shouldn't uplift this until we have a fix there too.

Comment 10

[Liz Henry (:lizzard)]

From irc conversation with sfink, we can hold off on this till 60.

Comment 11

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8953344 [details] [diff] [review]
Do not expect ion scripts when ion is unavailable

...except when I looked into bug 1426008, it turns out that it was already fixed in 59. So I'd like to re-request approval-mozilla-beta.

Comment 12

[Mike Hommey [:glandium]]

If this and bug 1426008 are fixed, can't we enable the tests on these builds now?

Comment 13

[Liz Henry (:lizzard)]

Comment on attachment 8953344 [details] [diff] [review]
Do not expect ion scripts when ion is unavailable

Great. let's uplift this fix to m-r for next week's RC build.

Comment 14

[Steve Fink [:sfink] [:s:]]

(In reply to Mike Hommey [:glandium] from comment #12)
> If this and bug 1426008 are fixed, can't we enable the tests on these builds
> now?

As long as something else hasn't broken as well, yes.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d43e9e56d922 (FIREFOX_59b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/7b78c4aa2d58