Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

annexB/B.2.1 and B.2.2 fail in node >= 0.11 (0.12, iojs) because 'escape' 'unescape' are non-enumerable #158

Closed
smikes opened this issue Feb 16, 2015 · 8 comments

Comments

@smikes
Copy link
Contributor

smikes commented Feb 16, 2015

Need some guidance on how to interpret Annex B.

TL;DR Should I submit a PR changing the tests to not require global.escape to be enumerable? Or should I submit a PR to node / iojs changing global.escape back to enumerable? Or is everything fine? /cc @bterlson @domenic and maybe @rwaldron

Note: this is an incorrect statement of the problem. More details below.

The ECMAScript language syntax and semantics defined in this annex are required when the ECMAScript host is a web browser. The content of this annex is normative but optional if the ECMAScript host is not a web browser.

When the ECMAScript host is a web browser the following additional properties of the standard built-in objects are defined.

The escape function is a property of the global object.

Current tests in test232/master require that escape and unescape be enumerable properties of the global object. As of node 0.11 (and all subsequent node variants, including 0.12 and io.js), these are now declared as enumerable.false:

node
> process.version
'v1.2.0'
> Object.getOwnPropertyDescriptor(global, "escape");
{ value: [Function: escape],
  writable: true,
  enumerable: false,
  configurable: true }

Causing the Annex B tests to fail. As I read the spec, the annex B tests don't apply to node at all – but is there any sort of requirement that if they choose to implement part of annex B, they must implement these functions as enumerable? Is there any requirement on browser implementers that these functions must be enumerable?

@bterlson
Copy link
Member

I guess precisely interpreting the spec language, node/io et al are free to do whatever they please since Annex B doesn't apply to them. That said, realistically it seems odd to implement the same behavior but with slight differences. In other words, if you're going to implement Annex B even if you're not required to, it should be per spec.

It sounds like the test failures are correct from a test262 perspective, though. The only addition would be to flag annex B semantics so we can filter them in the harness (which I believe is an open item at this point).

@anba
Copy link
Contributor

anba commented Feb 16, 2015

Current tests in test232/master require that escape and unescape be enumerable properties of the global object.

Which tests specifically?

@allenwb
Copy link
Member

allenwb commented Feb 16, 2015

"normative but optional" means that you aren't required to implement it, but if you do you are expected to conform to the provided specification.

I don't know why the current test is expecting enumerable: true. The ES6 spec. says in clause 17 "Every other data property described in clauses 18 through 26 and in Annex B.2 has the attributes { [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true } unless otherwise specified."

Regarding, should you implement it...everything is Annex B is something that we wish we could get rid of, but can't because of web legacy compatibility issues. If you think you an get away without implementing them, that's what I recommend you do. The more non-browser platforms that implement then, the harder it will be to ever get rid of them.

@smikes
Copy link
Contributor Author

smikes commented Feb 16, 2015

My mistake, the tests indeed specify enumerable: false and all node variants seem to report enumerable: false. What I'm running into must be a local problem, but I'm not sure what's causing it.

@smikes
Copy link
Contributor Author

smikes commented Feb 16, 2015

With io.js 1.2.0 I see this:

Svarog:test262 smikes$ node ../test262-harness/bin/run.js test/annexB/B.2.*
FAIL test/annexB/B.2.1.js
     Object.getOwnPropertyDescriptor returns data desc for functions on built-ins (Global.escape)
     Exp: no error
     Got: Error: Test case returned non-true value!

FAIL test/annexB/B.2.2.js
     Object.getOwnPropertyDescriptor returns data desc for functions on built-ins (Global.unescape)
     Exp: no error
     Got: Error: Test case returned non-true value!

Ran 11 tests
9 passed
2 failed
Took 0.371 seconds

While with the python runner (not updated to reflect needed changes in newer nodes, cf. bterlson/test262-harness#28 ) I see this:

Svarog:test262 smikes$ ./tools/packaging/test262.py --command node B.
annexB/B.2.1 passed in non-strict mode
=== annexB/B.2.1.propertyCheck failed in non-strict mode ===
--- errors ---  
 /private/var/folders/ml/dl5f4hk17bv0ht7tgxvvsw7c0000gn/T/test262-34LdCB.js:17
    throw new Test262Error(message);
          ^
Test262Error: #1: typeof this.escape !== "undefined"===
annexB/B.2.2 passed in non-strict mode
=== annexB/B.2.2.propertyCheck failed in non-strict mode ===
--- errors ---  
 /private/var/folders/ml/dl5f4hk17bv0ht7tgxvvsw7c0000gn/T/test262-AeRM8J.js:17
    throw new Test262Error(message);
          ^
Test262Error: #1: typeof this.unescape !== "undefined"===
annexB/B.2.3 passed in non-strict mode
annexB/B.2.4 passed in non-strict mode
annexB/B.2.4.propertyCheck passed in non-strict mode
annexB/B.2.5 passed in non-strict mode
annexB/B.2.5.propertyCheck passed in non-strict mode
annexB/B.2.6 passed in non-strict mode
annexB/B.2.6.propertyCheck passed in non-strict mode
annexB/B.RegExp.prototype.compile passed in non-strict mode

@smikes
Copy link
Contributor Author

smikes commented Feb 16, 2015

When I rewrite the test B.2.1.js to have separate assertions instead of 4 tests combined with &&, I am seeing a failure in test262-harness on >= 0.11, but success with 0.10.

Here's the test body I am using:

var global = fnGlobalObject();
var desc = Object.getOwnPropertyDescriptor(global, "escape");

assert.sameValue(desc.value, global.escape, "same as global escape");

assert(desc.writable, "writable");
assert.sameValue(desc.enumerable, false, "not enumerable");
assert(desc.configurable, "configurable");

I will try to check this on a different system, in case the real problem is that my laptop is hosed. If anyone has the time to check this, I'd be grateful.

@smikes
Copy link
Contributor Author

smikes commented Feb 17, 2015

Okay, I think I have a line on this now. There is an issue, but it's not between 0.10 and later versions, and it's not anything to do with whether escape is enumerable. I will close this issue and open one on test262-harness.

@smikes smikes closed this as completed Feb 17, 2015
@smikes
Copy link
Contributor Author

smikes commented Feb 17, 2015

Here's the regression between 0.10 and 1.2

0.10 -

$ node -p 'require("vm").runInThisContext("Object.getOwnPropertyDescriptor(this,\"escape\")")'
{ value: [Function: escape],
  writable: true,
  enumerable: false,
  configurable: true }
$ node -p 'require("vm").runInNewContext("Object.getOwnPropertyDescriptor(this,\"escape\")", {})'
{ value: [Function: escape],
  writable: true,
  enumerable: false,
  configurable: true }

iojs 1.2

Now using io.js v1.2.0
$ node -p 'require("vm").runInNewContext("Object.getOwnPropertyDescriptor(this,\"escape\")", {})'
{ value: [Function: escape],
  writable: true,
  enumerable: true,
  configurable: true }
$ node -p 'require("vm").runInThisContext("Object.getOwnPropertyDescriptor(this,\"escape\")")'
{ value: [Function: escape],
  writable: true,
  enumerable: false,
  configurable: true }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants