Closed Bug 907072 Opened 6 years ago Closed 6 years ago

ES6 reftests should run with "default version", not JS 1.8.5

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: wingo, Assigned: wingo)

References

Details

Attachments

(1 file, 2 obsolete files)

Currently the ecma_6 reftests are run with version(185).  

(In shells this is set via version(185) in ecma_6/shell.js, and in the browser there is a special case in browser.js that adds a version=1.8 onto the type=text/javascript for tests in the ecma_6 dir.)

Since ES6 features are designed to roll out to the web, under the default version, they should be tested as such.  Sometimes it's important to test the interaction between new ES6 features and existing default-mode behavior, as in generators.

Currently if the ecma_6 tests are run with version(0), the approximate trig/hyperbolic math tests fail:

REGRESSIONS
    ecma_6/Math/acosh-approx.js
    ecma_6/Math/sinh-approx.js
    ecma_6/Math/log1p-approx.js
    ecma_6/Math/expm1-approx.js
    ecma_6/Math/tanh-approx.js
    ecma_6/Math/hypot-exact.js
    ecma_6/Math/atanh-approx.js
    ecma_6/Math/asinh-approx.js
    ecma_6/Math/cbrt-approx.js
    ecma_6/Math/cosh-approx.js

We have a few possible solutions here:

  (1) Deploy these changes to Math to default version.  See bug 717391.

  (2) Revert these Math changes from SM and the ES6 spec.

  (3) Change browser.js to special-case ecma_6/Math only, not ecma_6 as a whole.

I think for bug 666399 I will do (3).  We should consider (1) if possible, and if not, then do (2).

WDYT?
Er, the Math bug I was referring to was 717379.  Sorry for the typo!
Sigh, linkable: bug 717379
Assignee: general → wingo
Comment on attachment 792682 [details] [diff] [review]
ES6 reftests should run with "default version", not JS 1.8.5

This patch does strategy (3), as a short-term measure.
Attachment #792682 - Flags: review?(jorendorff)
Attachment #792682 - Attachment is obsolete: true
Attachment #792682 - Flags: review?(jorendorff)
Comment on attachment 792685 [details] [diff] [review]
ES6 reftests should run with "default version", not JS 1.8.5 (v2)

Updated patch adds bug link to browser.js case.
Attachment #792685 - Attachment description: ES6 reftests should run with "default version", not JS 1.8.5 → ES6 reftests should run with "default version", not JS 1.8.5 (v2)
Attachment #792685 - Flags: review?(jorendorff)
Comment on attachment 792685 [details] [diff] [review]
ES6 reftests should run with "default version", not JS 1.8.5 (v2)

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

IIRC, the specific thing in those tests that causes them to fail under version(0) is the use of `let`.

    for (let [x, y] of cosh_data)
        assertNear(Math.acosh(x), y, sloppy_tolerance);

That looks ES6-legal to me under the current draft (?), but we don't support it in version(0) right now and the spec may change.

But you could just change `let` to `var` throughout that directory and remove the special case, r=me.
Attachment #792685 - Flags: review?(jorendorff) → review+
Attachment #792685 - Attachment is obsolete: true
Comment on attachment 792788 [details] [diff] [review]
ES6 reftests should run with "default version", not JS 1.8.5 v2

Good eye spotting the "let"!  I thought for some reason it was a Math thing.  Updated patch changes Math tests to run with the default version.
Attachment #792788 - Attachment description: ES6 reftests should run with "default version", not JS 1.8.5 → ES6 reftests should run with "default version", not JS 1.8.5 v2
Attachment #792788 - Flags: review+
Keywords: checkin-needed
looks good.
https://hg.mozilla.org/mozilla-central/rev/f081bd74d489
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.