test: add known_issues test for #10223 - #11024
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
const?
Sorry, something went wrong.
There was a problem hiding this comment.
This will be flagged by the linter. It additionally flags that util on line 7 is never used.
Sorry, something went wrong.
There was a problem hiding this comment.
Done, thanks
Sorry, something went wrong.
There was a problem hiding this comment.
Could you use single quotes around "foo" on this line and the next please.
Sorry, something went wrong.
There was a problem hiding this comment.
Done, thanks
Sorry, something went wrong.
bnoordhuis
left a comment
There was a problem hiding this comment.
LGTM with a suggestion.
Sorry, something went wrong.
There was a problem hiding this comment.
I'd return the descriptor and then check that .writable, .enumerable and .configurable are all false.
Sorry, something went wrong.
There was a problem hiding this comment.
suggestion incorporated:)
Sorry, something went wrong.
There was a problem hiding this comment.
ah, I just added the check for 'writable'. As a way to compare it all at once, would that work?:
const code = `
Object.defineProperty(this, 'foo', {value: 5});
Object.getOwnPropertyDescriptor(this, 'foo');
`;
const desc = vm.runInContext(code, context);
const foo = {value: 5, writable: false, configurable: false, enumerable: true}
assert.strictEqual(desc, foo)
Sorry, something went wrong.
There was a problem hiding this comment.
If you use assert.deepStrictEqual(), it should.
Sorry, something went wrong.
There was a problem hiding this comment.
I tried, but I don't think assert.deepStrictEqual() is working as intended...
I tested on a positive example and it throws an error:
const desc = vm.runInContext(code, context);
//desc: { value: 5, writable: true, enumerable: true, configurable: true }
const foo = {value: 5, writable: true, configurable: true, enumerable: true};
assert.deepStrictEqual(desc, foo); // throw new assert.AssertionError
Sorry, something went wrong.
PR-URL: #11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: #11024 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Checklist
Affected core subsystem(s)
tests
Description of change
Test addressing #10223 is added to the known_issues directory.
Writable attribute should be false by default.
In v6+ (reverted 524f693) it is set to true irrespective of whether
it was set to false or left out in Object.defineProperty call.
It will be fixed with the 5.5 V8 API changes