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

pEvent does not reliably return when used on same key twice #27

Closed
jwalton opened this issue Sep 26, 2019 · 5 comments
Closed

pEvent does not reliably return when used on same key twice #27

jwalton opened this issue Sep 26, 2019 · 5 comments

Comments

@jwalton
Copy link

jwalton commented Sep 26, 2019

I'm trying to use p-event in a test case. I have a key-value implementation which is watching a key. There should be a 'change' event fired when I create the watch, and then a second event fired if the key changes:

import * as promiseTools from 'promise-tools';
import pEvent from 'p-event';

        it('should watch a key', async function() {
            const stub = sinon.stub();

            await kv.setKey('foo', { foo: 1 });

            const watch = kv.watchKey('foo');
            watch.on('change', stub);

            await promiseTools.whilst(() => stub.callCount !== 1, () => promiseTools.delay(100));

            await kv.setKey('foo', { foo: 2 });

            await promiseTools.whilst(() => stub.callCount !== 2, () => promiseTools.delay(100));

            sinon.assert.calledTwice(stub);
            sinon.assert.calledWith(stub, { watch, key: 'foo', value: { foo: 1 } });
            sinon.assert.calledWith(stub, { watch, key: 'foo', value: { foo: 2 } });
        });

        it('should watch a key', async function() {
            await kv.setKey('foo2', { foo: 1 });

            const watch = kv.watchKey('foo2');
            const e1 = await pEvent(watch, 'change');
            expect(e1).to.eql({watch, key: 'foo2', value: {foo: 1}});

            await kv.setKey('foo', { foo: 2 });

            console.log('here');
            const e2 = await pEvent(watch, 'change');
            expect(e2).to.eql({watch, key: 'foo2', value: {foo: 2}});
        });

These two test cases ought to be equivalent, but the first one passes reliably, and the second one prints "here" and then times out waiting for the second await (but passes rarely). Am I missing something here?

@kevva
Copy link
Contributor

kevva commented Sep 27, 2019

You need to attach the event listener before the event is emitted. If the change event is emitted upon creating watch and pEvent is called after, it won't catch the first one. I don't know why it's working in your first test though, but it sounds sketchy. The second test "works" (or not) as expected however.

const emitter = createSomeEmitter();

emitter.emit('foo');

// Will never be resolved
await pEvent(emitter, 'foo');
const emitter = createSomeEmitter();

// Will be resolved
await pEvent(emitter, 'foo');

emitter.emit('foo');

@kevva kevva closed this as completed Sep 27, 2019
@jwalton
Copy link
Author

jwalton commented Sep 27, 2019

kv.watchKey() essentially does:

    watch = new EventEmitter();
    process.setTimeout(() => watch.emit('change'), 0);
    return watch;

So I have time to add a listener before the event is emitted.

@jwalton
Copy link
Author

jwalton commented Sep 27, 2019

Oh... I wonder if the library I'm using to talk to etcd is being overly clever, and emitting a change event right when I call setKey instead of waiting for etcd to send us the change. That would explain the behavior.

@kevva
Copy link
Contributor

kevva commented Sep 27, 2019

Yeah, probably. I just tried this and it passed:

const createEmitter = () => {
    const e = new EventEmitter();
    setTimeout(() => e.emit('foo', 'bar'), 0);
    return e;
};

const emitter = createEmitter();
const result = await pEvent(emitter, 'foo');

t.is(result, 'bar');

@jwalton
Copy link
Author

jwalton commented Sep 27, 2019 via email

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

2 participants