fix tape: test emitter in async function

[coderaiser]

Would be amazing if Promise not marked end of the test because it breaks such codeб calling end twice:

const {EventEmitter} = require('events');
const test = require('../../');

test('async6: double end', async function myTest(t) {
    const events = new EventEmitter();

    events.once('change', () => {
        t.pass('good');
        t.end();
    });

    setTimeout(() => {
        events.emit('change');
    });
});

Using subscription to events is a very common situation for end to end tests :). Anyways I want to thank you for a v5.0.0 and for a fix of rejected promises, I used try-to-tape to fix this behavior :).

[Raynos]

Auto calling "end" is a nice feature but breaks tests mixing promises and callbacks.

Honestly, mixing promises and callbacks is bad.

You should try writing tests like

test('async6: double end', async function myTest(t) {
    const events = new EventEmitter();

    setTimeout(() => {
        events.emit('change');
    });

   await new Promise((resolve) => {
       events.once('change', () => {
            t.pass('good');
            resolve();
        });
    })
});

[coderaiser]

Auto calling "end" is a nice feature but breaks tests mixing promises and callbacks.

Actually, if I want this feature I would take any other test runner 😄 :

• zora
• jest
• ava
• mocha
• tap

All of them gives ability of auto calling "end". One of the things I like in tape is ability to explicitly set ending of a test.

Honestly, mixing promises and callbacks is bad.

Could you please describe in more details why it's bad? My code is shorter, and I test this way a lot modules, and everything works good: working well tested simple to support code.

Also that was illustrative example of course I would rather choose such way of code:

import EventEmitter, {once} from 'events';
import holdUp from '@iocmd/hold-up';
import test from 'tape';

test('async6: double end', async (t) => {
     const emitter = new EventEmitter();
     const emitChange = emitter.emit.bind(emitter, 'change');

     await Promise.all([
         once(emitter, 'change'),
         holdUp(emitChange),
     ]);

     t.pass('good');
     t.end();
});

Using once and hold-up, but I can't change all my codebase in a moment :), but I would like to use latest version of tape with rejects fixed, for example :)

And also this is details of realization tape, it's leaking abstraction, and all this just to have ability not write end :)?

[Raynos]

This was discussed in the PR that added async function support ( https://github.com/substack/tape/pull/472#issuecomment-505484503 ).

Specifically https://github.com/substack/tape/pull/472#issuecomment-506275358

As a work around you can add

test('foo', async function t(assert) => {

  await new Promise((resolve) => {
    assert.once('end', resolve)
  })
})

Basically for any test where you do not want auto end behavior add an await on the end event at the bottom of the test.

[coderaiser]

OK, I understood that it is a well designed feature to use one promise for a full test lifecycle. Anyways it makes test harder to write and maintain, and breaks existing well-working tests. As a workaround supertape can be used, it is already extends tapes features list.

[Raynos]

If you want to make tests easier to write and maintain then dont use async/await in a callback driven test.

Aka

const {EventEmitter} = require('events');
const test = require('../../');

// Use vanilla function without async / await for testing callbacks.
test('async6: double end', function myTest(t) {
    const events = new EventEmitter();

    events.once('change', () => {
        t.pass('good');
        t.end();
    });

    setTimeout(() => {
        events.emit('change');
    });
});

[ljharb]

Indeed; these were all explicit decisions made.

If your test is an async function, the nature of async function is that it returns a promise, and so your entire test ends when that promise fulfills - which is how all other popular test runners work when receiving a promise.

I'm going to close this, since all the removed tests were added for a reason :-)

[Raynos]

This is a good example of breaking people's existing tests on tape @ 4 in the wild.

Adding a section to the README about migrating to v5 and end() already called might be useful.

[coderaiser]

@ljharb that's an interesting api change and it has sense. Would be great if you write about such changes in release notes, so consumers can read about what changes is waiting for them, and why :). Also message end is already called is not not intuitive :), message describing promise behavior with a link to issue would be much better (and deprecation notice in v4, so people can upgrade more smoothly).

[ljharb]

@coderaiser https://github.com/substack/tape/releases/tag/v5.0.0 is the release notes; the amount of prose that would be necessary to avoid all misunderstandings in any project is potentially very massive, and it's not imo reasonable to expect that :-)

Separately, v4 is NOT deprecated - deprecation messages are not simply to encourage updates. v4 will work fine likely forever, and there's no reason anybody needs to upgrade.

[coderaiser]

Separately, v4 is NOT deprecated - deprecation messages are not simply to encourage updates. v4 will work fine likely forever, and there's no reason anybody needs to upgrade.

@ljharb I didn't say that v4 is deprecated, I say that ability to work with async code, the old behavior is deprecated :).

the amount of prose that would be necessary to avoid all misunderstandings in any project is potentially very massive, and it's not imo reasonable to expect that :-)

Here is an example of release notes with well defined breaking changes :), I also have a lot packages, but that doesn't mean that breaking change I make should be invisible, if you have new vision on the way how library should be used, it's a good practice to describe this vision in some convenient way (I admit that our dialog can be this way, I'm OK with it, but that's no scaled).

Anyways I appreciate new knowledge I received today, and now my tests looks this way :).

[ljharb]

The ability to mix a Promise-returning test callback, with non-Promise code, has always been deprecated from a community best practices sense ¯\_(ツ)_/¯

[coderaiser]

Just created codemod to simplify transition to tape v5.0.0: convert-emitter-to-promise. It can help to refactor mixed callbacks with promises :).