Some improvements to well-known intrinsic objects harness file#4392
Some improvements to well-known intrinsic objects harness file#4392
Conversation
linusg
left a comment
linusg
left a comment
There was a problem hiding this comment.
Looks like a nice improvement!
| source: 'isNaN', | ||
| }, | ||
| { | ||
| name: '%IteratorPrototype%', |
There was a problem hiding this comment.
this intrinsic name will still be in the spec, so we should probably still have it in this list?
There was a problem hiding this comment.
It is no longer there as far as I can tell. It seems to have been replaced by %Iterator.prototype% which means you can get it in this case with getWellKnownIntrinsic('%Iterator%').prototype
There was a problem hiding this comment.
hmm, that change was my PR, and the intention was to leave intact any pre-existing intrinsics, so that might be a spec bug I’ll have to fix.
There was a problem hiding this comment.
I think your PR left %IteratorPrototype% intact, but it was subsequently replaced by tc39/ecma262#3395 (Iterator Helpers stage 4).
I did check if it was used by any existing tests (it isn't, except for Function/prototype/toString/built-in-function-object.js, which walks all of the properties of all well-known intrinsic objects and therefore will visit %Iterator.prototype% anyway via %Iterator%.)
So I think it's OK to just say that the entries here should match the entries in the table https://tc39.es/ecma262/#table-well-known-intrinsic-objects. Sound good?
As far as I know, %AsyncFromSyncIteratorPrototype% and
%ForInIteratorPrototype% are not (and not intended to be) accessible to
ECMAScript user code, so they are impossible to test directly. Add a
clarifying comment, and make the source expression consistent ('' vs.
'undefined').
As far as I can tell these are wrong, giving %GeneratorFunction.prototype% and %AsyncGeneratorFunction.prototype% instead. These new expressions are how MDN claims you can get the intrinsics.
Looks like this list hasn't been updated in a while. Add %AsyncGeneratorPrototype%, %GeneratorPrototype%, %Iterator%, %IteratorHelperPrototype%, and %WrapForValidIteratorPrototype%. %IteratorPrototype% is no longer a well-known intrinsic; I guess it was removed because ever since iterator helpers it's accessible as %Iterator.prototype%. %Iterator% is available as the global property Iterator, but include a fallback for implementations that haven't yet implemented iterator helpers.
wellKnownIntrinsicObjects.js now exposes a getWellKnownIntrinsicObject() function which returns the object corresponding to a key like %Array%. If the object is not provided by the implementation, or not accessible, it throws a Test262Error. This is so that tests depending on that intrinsic object can easily fail.
The objects it provides are also available in another harness file, wellKnownIntrinsicObjects.js. There's no point in duplicating that in the harness. Rewrite each test that used hidden-constructors.js to use getWellKnownIntrinsicObject instead.
ptomato
force-pushed
the
well-known-intrinsic-objects
branch
from
a4733ba to
02ceb7d
Compare
|
I'm taking the 👍 on the comment to mean my suggestion is to everyone's satisfaction 😄 |
4 participants
We have a
harness/wellKnownIntrinsicObjects.jshelper to aid with testing well-known intrinsic objects. There were some issues with it:harness/hidden-constructors.jsfile that duplicated some of the functionalityHere's an attempt at fixing this up a bit.
It doesn't convert any existing tests to use
getWellKnownIntrinsicObjectother than the ones that previously usedharness/hidden-constructors.jswhich is now removed. If we like this API, I'll go on and update more existing tests to use it.