Fate of promises-spec tests? about native-promise-only

[getify]

I originally got involved because...

Your involvement has been greatly appreciated, and I'm sure I speak not only for myself but also the other stakeholders @domenic @allenwb. It's a huge step forward. Thank you!

What do you want with this PR?

I haven't had a chance to review the current state of the tests, but I still plan to take a look soon. From what I recall as todo's (you may have already addressed some or all of these):

1. Check to make sure that the only tests in there are tests of the Promise(..), Promise.race(..), Promise.all(..), Promise.resolve(..) and Promise.reject(..) APIs.

   Any other tests which are testing conditions of a promise instance's behavior itself, such as then(..), are strictly unnecessary as that would be duplicative work of the Promises/A+ test suite. We need to remove any of those tests.

2. Separate the test suite that we do have into two "layers" as I was calling them:

   • Layer 1 is purely the tests of behavior that is exposable, and thus testable, in ES5. This is the stuff that's reasonable to test against polyfills like NPO. It's important to note that this layer will come close to ES6 spec compliance in many ways, but it's not strictly ES6 compliant (as it can't be). We probably need to figure out what to name it and call it so that it's not misleading/confusing about that nuance. Needs more thought.
   • Layer 2 is an optional set of additional tests that when combined with Layer 1 does attain full ES6 compliance. This full ES6 compliance is reasonable to test in ES6 compliant environments against, for example, the built-in Promise. This test suite layer must include testing ES6'isms like Promise[@@create], Promise.prototype, ES6 sub-classing, etc.

For the most cross-library usefulness, I think Layer 1 and Layer 2 need to be separately submitted to the promise-unwrapping repo, at least in such a way that someone can easily choose to run only Layer 1 or run both.

The combination of both layers is the test-suite that will eventually be submitted to Test-262, but that's actually something the promise-unwrapping repo should drive IMO, so we don't need to worry about that detail.

[getify]

I can make the three constructor tests pass by adding a little bit of logic to the beginning of function Promise()

I'm still holding off on making the change to NPO to get those constructor checks to pass. To me it's still an open question exactly what checks should be in Layer 1 vs. Layer 2, and I only care about making NPO pass Layer 1, obviously.

I have a strong feeling from that previous thread, but I don't think we had any final consensus, so please everyone double check me here if I'm missing anything:

1. Testing the constructor for whether the this is a "real" promise should not be based on this instanceof Promise checks, that much was clear. IOW, the check has to rely on some sort of branding, because there's just no other way to do it.

2. As for NPO's implementation specifically, one brand (with different shadowed values) can support two of the test cases with just one if statement:

   • on Promise.prototype, we need an .__npo__ brand, with value "uninitialized", which enables this assertion: Promise.call( {}, ..); // should throw. IOW, this.__npo__ === "uninitialized" has to pass.
   • on each promise instance, we need to shadow that brand by setting this.__npo__ to "initialized", which enables this assertion: Promise.call( new Promise(..), .. ); // should throw. IOW, this.__npo__ === "uninitialized" still has to pass. Same internal check, enables two different assertions in two different tests.

3. I think the conclusion was there's no practical way in the ES5 Layer 1 tests to check and enforce this third assertion: Promise.call( Object.create(Promise.prototype), .. ); // should throw.

   That test thus has to be moved to Layer 2, and needs to be checked in conjunction with testing Promise[@@create]. IOW, Promise.call( Promise[Symbol.create](), .. ) has to pass, and anything else you pass in has to fail.

If we can reach any kind of agreement on these observations, then I will patch NPO with the same.

[smikes]

I like the Layer 1 / Layer 2 framing, and to be honest I think we could even do with more granularity.

I have already grouped the tests by section of the spec. I could also separate them into layers (essentially all the tests written so far are layer 1 tests, because I am able to make them all pass using ES5 alone).

Even tests I have not yet implemented -- for example tests on the Promise prototype object (25.4.5) -- are in some sense Layer 1 in that a conforming polyfill could implement them in ES5.

Eventually it would be possible to use the test suite to make a claim like "X implementation passes Layer 1 tests for 25.4.3 (Promises), 25.4.4 (Promise object static functions), but NOT 25.4.5 (Promise prototype object static functions)"

[getify]

I think Layer 1 should be only the minimum required stuff for ES5. Prototypes and sub-classing are, IMO, inappropriate, as they cannot be implemented in ES5 while maintaining the spirit of external state immutability. As such, those tests belong to a Layer 2 test only.

I'm not sure I see why we need to have more granularity than that other than for trivia purposes, but I wouldn't object to it if there was a clean way to do it.

I just want to make sure there's a simple "layer 1" (or collection of "layer 1 sections") that fully exercises an ES5-only Promise implementation (sans sub-classing, sans Symbol.create, etc), and another layer or layer-collection for "everything else" which is for purely ES6 compliance and is appropriate only for ES6-only implementations (native or self-hosted).

I think it's quite possible, though I'm not sure off the top of my head, that grouping the tests by spec section number might not be adequate because there could be parts of some X.Y.Z spec section which are ES5 implementable and parts which can only be implemented/tested in ES6.

[smikes]

I think it's quite possible, though I'm not sure off the top of my head, that grouping the tests by spec section number might not be adequate because there could be parts of some X.Y.Z spec section which are ES5 implementable and parts which can only be implemented/tested in ES6.

They're independent dimensions. As I said, so far I've only implemented tests on ES5-visible functionality, across all the spec sections (25.4.3 - 25.4.5). At a later point, it would be possible to add essentially a whole second directory worth of tests, again organized by section, that hit the ES6-visible things like Symbol.create.

[getify]

I will trust your lead on that since you are much deeper in those details at the moment. If you think that's the right way to organize, it certainly matches how the Promises/A+ test suite is organized, so it makes sense.

Let's just make sure there isn't any limitation that makes it hard for a given library to pick out the tests that are applicable to its implementation.

[allenwb]

@getify

The combination of both layers is the test-suite that will eventually be submitted to Test-262, but that's actually something the promise-unwrapping repo should drive IMO, so we don't need to worry about that detail.

Actually in order to incorporate these into Test232 anyone who has contributed to these tests needs to go through the process of registering as a non-member TC39 contributor. http://www.ecma-international.org/memento/contribute_TC39_Royalty_Free_Task_Group.php . In addition to the on-line form on that page you need to fill in and sign the contributor software copyright license http://www.ecma-international.org/memento/TC39%20exhibit%20B.pdf

[getify]

@allenwb That's interesting. I didn't realize that our draft work here would require such coverage, since we don't intend to officially submit to TC39/Test262 directly, but rather submit to @domenic's efforts (who I presume is already covered) for his consideration.

I for one have no problem signing such an agreement, since I'm independent. I will have to leave it to @smikes to decide himself how that impacts his participation.

Thanks for clarifying what we need to think about.

[allenwb]

@getify Unless somebody else is paying for the work (or the author is otherwise contractually encumbered), the original author of any material is the copyright owner. For @domenic (really Google, now) to contribute you work to Ecma you would first have to license and/or transfer you copyright interest to them. Who knows what the process for that might me.

The easiest path is for everybody who is authoring tests to register as a TC39 contributor.

[smikes]

I wrote the tests and I am happy to sign those agreements, and I can
definitely get employer approval.

If that would be useful to contribute towards test-262. I have not yet
looked at the test-262 repository to see how compatible the test frameworks
are.

[smikes]

1. CLA - I have done the online portion and have completed the written portion, ready to go in tomorrow's mail.
2. I have now installed and tested against node 0.11 (which includes a version of v8 that implements Promise with known problems.) This test shows two failed assertions which are due to errors in my tests. I will fix those errors and commit a change to this branch.

[smikes]

@allenwb I looked at the Test262 code and there are two problems, neither insurmountable, with adding what I have:

1. Test262 does not currently seem to have any asynchronous tests. Although many of the tests I have written can be run synchronously (static tests and claims about the presence or arity of particular functions), some tests do require asynchrony. Someone more familiar with test262 should choose a framework for async tests.

2. I am not familiar enough with ES6 to write idiomatic ES6 tests. Notably, when using node 0.11.13 (a recent development version of Google's v8) attempting to instantiate a promise "manually" via Promise.call(Object.create(Promise.prototype), function () {}) throws the remarkable error TypeError: #<Promise> is not a promise

   1) 25.4.3 The Promise Constructor can be called as a function:
     TypeError: #<Promise> is not a promise
   at Z:\Code\github\native-promise-only\test\tests\constructor.js:17:14
   at Function._throws (assert.js:293:5)
   at Function.assert.doesNotThrow (assert.js:324:11)
   at Context.<anonymous> (Z:\Code\github\native-promise-only\test\tests\constructor.js:16:9)
   at callFn (Z:\Code\github\native-promise-only\node_modules\mocha\lib\runnable.js:223:21)
   at Test.Runnable.run (Z:\Code\github\native-promise-only\node_modules\mocha\lib\runnable.js:216:7)
   at Runner.runTest (Z:\Code\github\native-promise-only\node_modules\mocha\lib\runner.js:373:10)
   at Z:\Code\github\native-promise-only\node_modules\mocha\lib\runner.js:451:12
   at next (Z:\Code\github\native-promise-only\node_modules\mocha\lib\runner.js:298:14)
   at Z:\Code\github\native-promise-only\node_modules\mocha\lib\runner.js:308:7
   

I believe that I am seeing this because Object.create() is not the correct ES6 way to call Promise[ @@create ](), but I am also not sure what the correct way is -- besides of course new Promise()

[getify]

I believe that I am seeing this because Object.create() is not the correct ES6 way to call Promise @@create , but I am also not sure what the correct way is -- besides of course new Promise()

Correct, Object.create(..) is very different from @@create.

In a conforming ES6 implementation (dunno if v8 in node 0.11.x is or not in this respect), IIUC, you would call it like this:

Promise.call( Promise[Symbol.create](), .. );

[smikes]

It appears that the version of v8 currently shipping in node 0.11.13 does not handle that part of ES6 yet, then:

> Symbol.create
TypeError: Object.keys called on non-object
at Function.keys (native)
at formatValue (util.js:235:21)
at Object.inspect (util.js:147:10)
at REPLServer.self.writer (repl.js:209:19)
at finish (repl.js:316:38)
at REPLServer.defaultEval (repl.js:145:5)
at bound (domain.js:257:14)
at REPLServer.runBound [as eval] (domain.js:270:12)
at REPLServer.<anonymous> (repl.js:277:12)
at REPLServer.EventEmitter.emit (events.js:107:17)

[smikes]

With that test removed, I now have a suite of tests which passes on v8 promises -- as long as I do not include the promises/A+ tests, which are known to fail on node 0.11.13 because of bugs in the Promise implementation that is included.

In particular, the three constructor tests from 25.4.3.1 are all satisfied by v8 Promise:

1. throws TypeError when 'this' is not of type Object
2. throws TypeError if 'this' is a constructed by unresolved Promise
3. throws TypeError if 'this' is a resolved Promise

Tomorrow if I have some time I will try to run against the Mozilla implementation of ES6 Promises.

[allenwb]

@smikes There is an on-going discussion about Test262 async tests, see tc39/test262#34

In general, you should coordinate with @bterlson on Test262 issues.