Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes having to hit CTRL + C when doing serial tests with Socket.io #148

Open
wants to merge 2 commits into from

4 participants

@ignacioiglesias

I was doing some tests using Socket.io and even though everything ran okay except that I had to manually exit Expresso using ctrl + c.

I tried using process.exit inside the else block but that would cause Expresso to output Failure: Only 1 of 3 suites have been started. Using a timeout seems to fix it despite the fact that I'm using 0 as time. Not really sure how this works, but it does :)

Edit: The first answer of "Why does setTimeout(fn, 0) sometimes help?" provides a good explanation.

BTW, this it my first push request, I hope it helps.

Juan Ignacio Iglesias Fixes having to hit CTRL + C when doing serial tests.
I was doing some tests using Socket.io and even though everything ran
okay except that I had to manually exit Expresso using ctrl + c.

I tried using `process.exit` inside the `else` block but that would
cause Expresso to output `Failure: Only 1 of 3 suites have been
started`. Using a timeout seems to fix it despite the fact that I'm
using 0 as time. Not really sure how this works, but it does :)
ec2da8a
@ignacioiglesias

process.nextTick can (should?) be used instead of setTimeout. Sorry about that :/

@AutomatedTester

Hi,

When do we think this pull request will be merged, as well as a release? Currently this bug is blocking me from using expresso in a CI because it locks the worker up. See http://travis-ci.org/#!/AutomatedTester/powerball-platform/builds as an example

@michaelklishin

Like @AutomatedTester has said, this is a pretty annoying issue for us at travis-ci.org. Some otherwise good test suites may take up to 10-30 minutes (depending on the timeout value) to be killed after they get stuck because of this bug.

@ignacioiglesias

Hey guys, just out of curiosity which versions of Node are you using?

@tj
Owner
tj commented

it's not really a bug, it's just an event-loop gotcha. It can only be "fixed" if we specify that each test is done and then exit manually. the successor to expresso does this

@ignacioiglesias

Is that successor the beforeExit or next argument that is provided when using --serial?

@AutomatedTester
@tj
Owner
tj commented

expresso doesn't make you notify it when you're done (unfortunately) so we can't really auto-exit unless we add that and everyone updates all the tests. I'll release mocha soon when I have some time to finish it, there's an interface very similar to expresso so if anyone wants to migrate it should be pretty painless

@ignacioiglesias

Uhm, not sure if I understood…
My thought was that I told expresso that my test had finished by calling that beforeExit or next argument; therefore, once expresso had ran out of files (in line 792) it would finish the process.

@tj
Owner
tj commented

beforeExit doesn't really help but yeah when --serial next() is enough, but not when running parallel

@ignacioiglesias

Sorry for my late answer.

I think that the problem was having to hit CTRL + C when testing in --serial mode. Even though I was calling next() when finished, I'd still had to manually exit.

Of course that, without --serial, next() will not be available; but I thought this could be useful to fix at least part of this gotcha.

Anyway, feel free to close this :)

Thanks for your help, TJ!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 29, 2011
  1. Fixes having to hit CTRL + C when doing serial tests.

    Juan Ignacio Iglesias authored
    I was doing some tests using Socket.io and even though everything ran
    okay except that I had to manually exit Expresso using ctrl + c.
    
    I tried using `process.exit` inside the `else` block but that would
    cause Expresso to output `Failure: Only 1 of 3 suites have been
    started`. Using a timeout seems to fix it despite the fact that I'm
    using 0 as time. Not really sure how this works, but it does :)
Commits on Oct 30, 2011
  1. Replaced setTimeout with process.nextTick

    Juan Ignacio Iglesias authored
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 0 deletions.
  1. +4 −0 bin/expresso
View
4 bin/expresso
@@ -791,6 +791,10 @@ function runFiles(files) {
(function next() {
if (files.length) {
runFile(files.shift(), next);
+ } else {
+ process.nextTick(function() {
+ process.exit(0);
+ });
}
})();
} else {
Something went wrong with that request. Please try again.