Working on tests for Promise.all about native-promise-only

[smikes]

I have now completed tests for Promise.all and Promise.race, and these functions are now fully covered.

I have also started to add tests for the other Promise properties described in the latest (May 27 2014) ES6 draft document.

So far it's quite specific to this polyfill; if the goal is to create a general ES6 Promise test suite in the manner of promises-aplus-tests, then a few things should happen:

1. These tests should be split off into its own repository and npm module
2. These tests should not depend on nodejs-specific features such as require() and setImmediate() (which they currently do, though I am trying to isolate that into a 'helpers.js' module)
3. These tests should use a browser-friendly assertion library, e.g., should -- instead of using node 'assert'

Eventually these tests should work against a Promise native implementation in a browser or in node, and also against a Promise polyfill. At present they only work against this polyfill, and will require some refactoring to work elsewhere.

[getify]

started to add tests for the other Promise properties described in the latest (May 27 2014) ES6 draft document.

Can you please list in detail what other properties you mean here? Here's what I know should be being tested for compliance to ES6 spec:

• Promise(..)
• Promise.resolve(..)
• Promise.reject(..)
• Promise.race(..)
• Promise.all(..)

What sorts of tests are you making for other properties? Like testing the Promise.prototype or something like that?

For some of that stuff, there will be some hairy parts. For example, I don't think it's practical or wise to create tests about Promise sub-classability (which moreover would fail with this polyfill -- see Limitations), because I think it's basically impossible to fully polyfill a (performant) trustable promise mechanism that's also sub-classable in the traditional sense. So I'm not sure if tests against Promise.prototype are germane to this effort. I think sub-classability is a topic all unto its own and should be handled more at the ES6 level, for all built-ins, and left out of scope for our purposes.

At the very least, if we construct tests around Promise.prototype, we have to make those tests optional or configurable to be turned off or something.

Also, I wish there was a way to test that promises are actually trustably externally immutable, but that's basically impossible since you can't know in what ways someone makes external state visible. A test for a promise instance having no other properties besides then and catch would theoretically work (at least sorta) but then it'd be very brittle and almost all promise libs (besides this one!) would break. So that's also probably not a reasonable test, even though I deeply wish it could be.

For now I think we should stick to testing the behaviors of the constructor and 4 static method helpers. If you have anything else to test, let's discuss it before going too much further down that route.

---

I would ideally like to copy the same test infrastructure as the Promises/A+ Test Suite, because I think that will make these Promise.* tests most easily integrate-able by other promise projects which are already A+ compliant. Those tests do a good job, it seems, of being cross-environment capable, and are also very thorough.

So far it's quite specific to this polyfill

Can you elaborate very specifically on what about the tests is "quite specific to this polyfill"? I agree we should work to isolate the tests from any such specifics. The A+ test suite uses the "test_adapter.js" thing to isolate any setup specifics, which I think we should copy. For a true polyfill like this one, that adapter should be nothing more than loading up the native-promise-only polyfill then letting the tests naively find and use the global Promise it patches in.

If we can get a list of what you think is wrong/difficult here, we can work to fix it.

One challenge to note is that it's impossible to test this polyfill (in its truest sense) in an environment that doesn't need the polyfill (aka, is already ES6 standard). In those cases, the test suite would just run against the native implementation. A polyfill by definition cannot, IMO, be instructed to override the native implementation (unless it were deeply broken, but that's another topic entirely).

These tests should be split off into its own repository and npm module

I think the eventual goal is to have a test-suite that can be moved into this project: https://github.com/domenic/promises-unwrapping But we don't need to worry about those details right now, let's just get the substance of the tests down.

These tests should not depend on nodejs-specific features such as require() and setImmediate() (which they currently do, though I am trying to isolate that into a 'helpers.js' module)

Agreed, see above.

These tests should use a browser-friendly assertion library, e.g., should -- instead of using node 'assert'

See above about "Promises/A+ Test Suite" -- whatever they've done, let's follow their lead.

Eventually these tests should work against a Promise native implementation in a browser or in node, and also against a Promise polyfill.

Yes, agreed. As stated above, if native-promise-only is in some way not behaving (with respect to the above list and limitations, of course) like native, such that tests are reliant on the polyfill, then that needs to be fixed ASAP.

At present they only work against this polyfill, and will require some refactoring to work elsewhere.

Again, please be specific about any test-reliance on the polyfill itself. We want to avoid that from the beginning, so we should address that and resolve it before moving anything else forward.

[smikes]

That's a big comment. Let me break it up:

First- specific to this polyfill, there are two explicit dependencies.

1. I am explicitly loading the source-version of the polyfill with require("../lib/npo.src.js")
2. I am reviewing coverage against the polyfill

This is not heavily entangled, but it is something I am highly aware of. Eventually I want to refactor in a way similar to the adapter system used by the Promises/Aplus test. That will be a simple commit, one line per source file, more-or-less.

[smikes]

Regarding the assertion library - promises-aplus-tests uses require("assert") so I am doing the same.

I am nervous, though, about how that plays out with mocha in the browser. I am not very familiar with frontend testing and I don't want to wind up writing a lot of assert() calls that need to be changed to something else. Is require("assert") safe in the browser?

[getify]

Is require("assert") safe in the browser?

That's a question best directed at the promises/a+ test suite project. What's good for them is good for us. If they're not handling that detail yet (I know they are somehow, because I know the test suite can run in the browser) then we should resolve those details there, then copy that with what we do here. :)

[smikes]

Regarding the tests on Promise.prototype, I have merely stubbed them in for now. (Mocha identifies them as "pending"). They are pretty vanilla -- things like Promise.prototype.constructor exists and is an object, Promise.prototype.then is a function, etc. I will leave them stubbed for the time being.

[smikes]

One thing that's worth doing (later!) after the basic Promise. and Promise.prototype tests are written, would be to create some stress tests where various ES5/ES6 features get thrown at the Promise library.

The simplest example I'm thinking of is that ES5 allows the user to preventExtensions, seal, and freeze an object (and a Function is an object), so Promises need to work even when the functions passed to the Promise constructor and to then are frozen, sealed, etc. Doesn't appear to be a problem in your polyfill, but some people may have 'cleverly' relied on their ability to add properties to users' objects... it would be good to identify problems like that early.

[getify]

it would be good to identify problems like that early.

This is exactly the sort of thing I was referring to above in my "big comment" about "trustably externally immutable" tests. In theory it sounds great to test, but in practice I think it's too brittle and too narrow.

Here's another way to think of it: the ES6 spec mandates what must be possible (the minimum), but it rarely dictates that you cannot do other things extra. Without thinking too deeply about this assertion, I am not aware of any restriction the spec itself makes in terms of promise objects having their state as completely public and mutable properties.

Now, the spec doesn't do anything to detail the spirit of promises, with respect to "trustably externally immutable" state. It doesn't say anything like "must not add properties to.." or anything like that.

So even though I personally think most of the libraries that are currently doing that are wrong, they are in fact not breaking the spec (only the spirit).

We should stay away from tests that try to narrow more than the spec does. We should stick to checking that a promise implementation at least does what the spec requires, and leave it to users, blog posts, books, etc to detail and decide the tradeoffs of the other stuff or even the how that various libs will choose.

Make sense?

[smikes]

Agreed. Effectively there are two regions of interest. First is to-the-letter-of-spec conformance in implementing things that the spec positively says about Promises. That's what we're aiming for now.

Second, and later, and potentially quite interesting, is trying to identify and exploit problems with interactions between different parts of the specs. (For example, once users can define their own iterators, Promise.all() and Promise.race() need to accept any Iterable, not merely arrays...)

I would argue that those tests are also a very important part of spec compliance, because nobody expects that taking their spec-conformant frozen (or custom Iterable, or whatever) code and using it with Promises will break. Of course there will be breakage, and it's better to know early. But: not this project, and not today.

[getify]

Yeah, I think those are important tests, but they're important for test-262 to handle. They're of a much bigger philosophy than we're dealing with here.

[domenic]

Good thread. A few things...

Regarding strategy for the evolution of the test suite: I mostly agree with @getify. Aligning with the Promises/A+ suite, and/or with promises-unwrapping, is the way to go for now. Eventually we will port toward test262, but that will require a number of things to happen. (Off the top of my head: test262 needs to start accepting contributions without a CLA that you literally have to mail to Switzerland; test262 needs to have a fully-working solution for async tests; and, for minimum development friction, someone needs to develop a test262 runner for Node.)

One thing that might make things easier is trying to keep the test structure flat, and minimize the use of meta-programming (i.e. tests that create tests). The existing Promises/A+ and promises-unwrapping tests fail these criteria badly (lots of nesting, lots of metaprogramming), which will I think make them harder to port to test262.

Regarding tests of subclassing: I think that's actually quite testable, but we can certainly leave those for later.

Regarding require("assert"): works great with browserify and related tooling, and it's the simplest option. Definitely use it. We'll deal with the migration to whatever test262 has later; I am not too worried.

Awesome work that you guys are doing. I hope over the next few weeks to have time to start coalescing and test262-ing stuff.

[smikes]

[ Moderator: I've cleaned up the formatting a little bit. :) ]

Replying via email, sorry if formatting is wack.

The tests are simple so far, using nesting (but only for grouping) with no
between-test dependencies. They use mocha's async.
They use require("assert").

I will flatten the hierarchy so there is a single "describe" per ES6
feature, and move some of my annotations into comments.

I am following a pattern of:
Lifting a single declarative sentence from the ES6 spec
Translating that into a test

Sometimes the a single line of the spec turns into 2-4 sentences.

Regarding strategy for the evolution of the test suite: I mostly agree with @getify. Aligning with the Promises/A+ suite, and/or with promises-unwrapping, is the way to go for now. Eventually we will port toward test262, but that will require a number of things to happen. (Off the top of my head: test262 needs to start accepting contributions without a CLA that you literally have to mail to Switzerland; test262 needs to have a fully-working solution for async tests; and, for minimum development friction, someone needs to develop a test262 runner for Node.)

One thing that might make things easier is trying to keep the test
structure flat, and minimize the use of meta-programming (i.e. tests that
create tests). The existing Promises/A+ and promises-unwrapping tests fail
these criteria badly (lots of nesting, lots of metaprogramming), which will
I think make them harder to port to test262.

[getify]

@domenic thanks for your thoughts. Very helpful.

Regarding tests of subclassing: I think that's actually quite testable, but we can certainly leave those for later.

There's a lot more to my assertions than just "is it strictly testable", but we should continue that discussion in another thread at some later time, and not bog this one down. :)

Note: we will be having tests for the this-borrowing of the 4 static methods and the then and catch methods, which is a subset of what one would expect in a sub-classable implementation, so at least there's that. See this thread for more info.