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

Promise support #262

Closed
kl0tl opened this issue Feb 18, 2016 · 7 comments
Closed

Promise support #262

kl0tl opened this issue Feb 18, 2016 · 7 comments

Comments

@kl0tl
Copy link
Contributor

kl0tl commented Feb 18, 2016

Would be great to support returning a promise to end the test when the promise is resolved.

  • It would play nice with future extensions of the language, allowing one to call tape with Async Functions to transparently return a promise.
  • It would eliminate some otherwise unfixable ordering issues with nested tests.
  • Rewriting
var test = require('tape');

test('first', function (t) {

  setTimeout(function () {
    t.ok(1, 'first test');
    t.end();
  }, 200);

  t.test('second', function (t) {
    t.ok(1, 'second test');
    t.end();
  });
});

test('third', function (t) {
  setTimeout(function () {
    t.ok(1, 'third test');
    t.end();
  }, 100);
});

to (assuming a function delay returning a Promise resolved after t milliseconds)

var test = require('tape');

test('first', async function (t) {

  await delay(200);

  t.ok(1, 'first test');

  t.test('second', async function (t) {
    t.ok(1, 'second test');
  });
});

test('third', async function (t) {
  await delay(100);
  t.ok(1, 'third test');
});

will preserve the expected order (see #222).


Looking at blue-tape it should be doable with something along the lines of

var ret = this._cb(this);
var self = this;
if (ret && typeof ret.then === 'function') {
  ret.then(function () {
    self.end();
  }, function (err) {
    nextTick(function () {
      throw err;
    });
  });
}

Note that it would not require a Promise library nor any configuration. Users wanting to take advantage of this feature would need to provide their own Promise implementation if necessary anyway.

@ljharb
Copy link
Collaborator

ljharb commented Feb 18, 2016

From #222 (comment):

Supporting that is tricky because we'd have to either a) assume Promise was available, which forces the user to know they need to shim it, b) allow explicit injection of Promise so they can pass bluebird or native Promises or whatever as they like (like .setPromiseConstructor() or something), or c) add a Promise lib as a dependency.

Solution a is dangerous, b is awkward, and c is heavyweight.

The problem is that tape would need to be able to wrap test return values in Promise.resolve since it's a bad idea to do an "is this a promise" check - which means tape needs to be able to know that Promise is available.

@kl0tl
Copy link
Contributor Author

kl0tl commented Feb 18, 2016

Why would wrapping be necessary ? We don’t need any of the capabilities of well behaved promises here. The success callback could even be called on the same tick and tests would still close properly.

@ljharb
Copy link
Collaborator

ljharb commented Feb 18, 2016

and if current tests return non-promise objects?

Absolutely we do need them - those capabilities (including error-catching) are part of the guarantee of promises. If the success callback is called on the same tick than a previously set-up "next tick" operation might not fire, and the test would be invalid.

@kl0tl
Copy link
Contributor Author

kl0tl commented Feb 18, 2016

Yeah I know about the guarantees promises offer but I think it's unlikely something too bad could happen here. Anyway, couldn't this behavior be namespaced under a new Test#async method to make the intent explicit ? Would it be acceptable to trust users to provide well behaved enough promises in that case ?

Doesn't tape do let errors propagate on purpose ? I'm not sure what you're meaning about error catching.

@ghost
Copy link

ghost commented Feb 18, 2016

This would make tape much more complicated. A promise-oriented test library should be a separate project. Tape is a callback-oriented project. I would much rather there were 2 projects focusing on separate, clear conceptions of what they are trying to be than a single monster of a project that mixes a bunch of ideas into an incoherent mess.

@ghost ghost closed this as completed Feb 18, 2016
@kl0tl
Copy link
Contributor Author

kl0tl commented Feb 19, 2016

@substack It's unfortunate to hold onto this callback based API, it seems to be the opposite of the direction the language is taking and its causing some weird issues. Anyway, thanks for taking the time to talk !

@levino
Copy link

levino commented Jun 25, 2019

Time to move to promises?

This issue was closed.
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

3 participants