Add configuration for timeout about chai-events HOT 11 CLOSED

@rijnhard Glad you've figured it out. Please do circle back to this issue if configuring the timeout would be useful! (Or if you figure out a way to reduce the hackishness of the timeout, I'd love suggestions!)

from chai-events.

This has been implemented in #13. The API has not been vastly changed as suggested in this issue.

from chai-events.

I'm running into this issue now, infact I'd say it shouldn't be set since you are returning a promise and the promise should be handled.
In the world of async and await it's even easier.

it('test', async function() {
    const ev = new EventEmitter(),
        p = ev.should.emit('x');

    // do magic

    await p;
});

The unconfigurable and hidden timeout is actually a bit too much magic IMHO, but removing it is a breaking change. But I would still suggest removing it and optionally adding its support with an after/before function. Then updating the docs to reflect this.

from chai-events.

@rijnhard Actually, behind the scenes, it always is using a timeout, even with a promise:

The promise just resolves when the timeout is finished, to relay to Mocha that the delay for the assertion is complete. In fact, I don't think the code works at all without awaiting p.

I'd love to get rid of the timeout, but I think the code needs to wait for some duration to ensure events are created/propagate before passing/failing.

from chai-events.

@CodeLenny I'm busy messing with it now, I did notice the timeout, which I thought was an issue so I implemented a hack using promises with no timeouts and then hit a full circle.

In my use case I'm using a library that needs to establish a connection to a remote application, and the library is quite stupid in that it blocks on the connect and doesn't fail correctly. So I would need a timeout anyway. Full circle.

So for me at least, there's no reason to change the behaviour now, and in fact it would stop tests from hanging even if not configurable.

from chai-events.

I'm going to need this so my tests in #7 don't take forever.

from chai-events.

Draft of API:

.then(), .catch(), and .promise() methods will execute the assertion, instead of immediately executing when .emit() is called.

However, this should not be new, as the return value from .emit() would need to be returned to Mocha, or passed to await to be meaningful. Never the less, this will represent a breaking change, and will cause a major version number increment.

New actions will be chainable after .emit():

(x).should.emit("foo").after(200).promise();
return (x).should.emit("bar");

@rijnhard I don't think this will get in your way, and it should be wrapped up quite shortly. However, do you want your PR changes merged as a minor version to avoid any breaking changes?

from chai-events.

@CodeLenny for other users I'd suggest making a minor bump, i'm not much phased.

The only comment I have is why have a .promise() method if it returns a Promise by default?

from chai-events.

@rijnhard Ah yes, I probably left out some of the details.

.emit() no longer will return a promise, and start listening to events. It merely configures settings inside Chai. This means that .after(), etc. can configure settings as well.

.then(), .catch() (and .promise) would be Chai methods/properties that would internally run what .emit() used to run (e.g. listening to the eventemitter, starting the "timeout timer", setting up assertions), and pass any arguments to the internal promise. So the result of .emit() and .after() looks like a promise, but in fact isn't actually a promise.

So in some cases, it works without fault:

it("listens", () => {
  setTimeout(() => socket.emit("foo"), 100);
  return socket.should.emit("foo").before(150);
});

Because Mocha calls .then() on the returned Promise, everything works as planned.

However, this might mess up current workflows (example from one of the tests):

it("listens", () => {
  const p = emitter.should.emit("foo");
  emitter.emit("foo");
  return p;
});

Before, .emit() would start listening to the emitter, so it would catch emitter.emit("foo");
With the current plan, the emitter only starts listening once p is returned to Mocha, and Mocha calls .then() on it, so it doesn't catch "foo". That's the breaking change I was referencing.

So, back to the original question: why .promise? In the last example, emitter.should.emit("foo") could be rewritten two ways to make it work like before:

function noop() { return true; }
const p = emitter.should.emit("foo").then(noop);
const p = emitter.should.emit("foo").promise;

.promise will execute the pipeline and return a promise (hence the name) without needing to pass an argument to the .then() proxy method.

I appreciate the feedback, thanks! If it was documented that .emit() and .after() return a Chai object, and you need to call .then(), .catch(), or .promise to get back a promise, would this make sense? Or should I think about redesigning stuff to make it more clear?

from chai-events.

mmmmmmmmm, my brain function is currently a bit sub-optimal, but let me try. I understand the design decision you made and can agree with it.

for my use cases its a lot of things like this:

it("listens", async () => {
  const p = emitter.should.emit("foo");
  // do magic 
  emitter.emit("foo");
  await p;

  // carry on with other assertions
});

I do a lot of behavioural testing so multiple assertions normally happen in an it.

making/keeping the return thenable is great, but in terms of when it starts listening, I do think the original way actually had a nicer DSL. With async/await it's pretty easy to structure it how you want.

In general with tests (especially behavioural) you want to define the expected behaviour before actioning whatever should/shouldnt create that behaviour. I'm struggling to see the value of the behaviour being checked AFTER the action, it kind of sounds like you are going to go around in circles with promises.

But do elaborate on the use cases where you think it would be useful, but for behavioural tests I don't see the value.

from chai-events.

barring of course using timeouts in after

from chai-events.