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

wait for the event queue to drain before starting the next step #500

Closed
kevgo opened this issue Jan 23, 2016 · 8 comments
Closed

wait for the event queue to drain before starting the next step #500

kevgo opened this issue Jan 23, 2016 · 8 comments

Comments

@kevgo
Copy link
Contributor

kevgo commented Jan 23, 2016

In order to support asynchronous clean-up better, and increase overall robustness of the framework, it would be useful if CucumberJS would start the next scenario only after waiting for the event queue to drain. What this means is do a setTimeout <continue>, 0 before running the next scenario. This gives any queued up asynchronous cleanup jobs from the previous scenario time to run before Cucumber starts with the next scenario.

@charlierudolph
Copy link
Member

Can you please give an example? I'm curious why asynchronous cleanup jobs that aren't performed in after hooks.

@kevgo
Copy link
Contributor Author

kevgo commented Jan 31, 2016

I have test code that makes network calls. My tests properly waits for the response to the call, and my SUT sends out the response only when it is done processing the request. So everything should be golden.

And it works fine. Sometimes, however, these calls get processed after the next scenario step is already running. I have no clue why this happens.

The solution to this is to add a "setTimeout 0" step to the next test step that uses the result of the network call.

This out-of-order processing of events may be an issue of OS X, maybe it's Node, maybe its a bug in CucumberJS, or maybe its an issue with my code. In any case, I think adding additional bulkheads to compartmentalize different sequential activities (like scenario steps) more from each other is a good architectural pattern for Node apps. CucumberJS should provide that pattern to make it easy to write robust tests that aren't plagued by various edge cases around timing and the Node event queue. Just a useful best practice for such a framework imho.

@charlierudolph
Copy link
Member

Your original request was only to do add a "setTimeout 0" after each scenario yet your use case appears to asking for it after each step. Please clarify.

Also having a "setTimeout 0" is only useful if an asynchronous operation has already completed and is waiting for a chance to trigger its callback. We aren't actually waiting for all stray asynchronous operations, which is what I think you are trying to solve. To me this solution actually has the potential to hide timing errors as you are relying on something completing in another event loop instead of explicitly waiting for it to complete.

@kevgo kevgo changed the title wait for the event queue to drain before starting the next scenario wait for the event queue to drain before starting the next step Feb 1, 2016
@kevgo
Copy link
Contributor Author

kevgo commented Feb 1, 2016

I'm proposing to wait for the next event loop iteration before starting the next step (I have fixed the issue title). The proper method for that seems setImmediate. Running it before each scenario allows any pending IO currently queued up in the event queue to be processed before starting the next step.

I agree that doing so hides timing errors in the test code. This is the whole point of the bulkhead pattern: to provide additional robustness against edge cases, so that the developer doesn't have to obsess about getting all the different timing issues right, and prevent timing issues from affecting other parts of the test suite.

@mete89
Copy link

mete89 commented Nov 16, 2016

I have similar problem. I'm using request module for checking some resources on CDN. Mostly, it skips result of promise and continue with next case.

What is the proper way of using async calls in cucumber.js?

@charlierudolph
Copy link
Member

@kevgo I'm going to close this as I disagree with the need for this. I really don't like the idea of writing a test to ensure this happening as it would make something work that I don't think should.

@mete89 ensure you return the promise if you want cucumber to wait for it complete. Docs. I'll add a comments making it more clear the return is necessary

@kevgo
Copy link
Contributor Author

kevgo commented Dec 12, 2016

Fine with me, I don't feel strongly about it. :)

@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants