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

Make sure the server process dies on error #75

Merged
merged 2 commits into from
Aug 29, 2012
Merged

Conversation

mutru
Copy link
Contributor

@mutru mutru commented Aug 24, 2012

It's possible that a server process is spawned and it is alive, even if runner_available? returns false. This change calls Server.stop to make sure that the server won't hang around forever.

This caused an issue with our CI server, where one failed test run left a child process that held the listening socket, and further test runs would fail.

I also fixed a small typo in the 'without the runner available' spec.

Otto Hilska added 2 commits August 24, 2012 00:02
It didn't actually test anything, because it stubbed the wrong method name.
It might be that the server process got started, even if we weren't able
to connect to it. For example, sometimes bootstrapping the environment
may take too long.
@travisbot
Copy link

This pull request passes (merged db095a3 into c79ae41).

@mutru
Copy link
Contributor Author

mutru commented Aug 24, 2012

Hold your horses, it might be that the fix introduced another issue. Let me test it a bit more.

@netzpirat
Copy link
Contributor

This will not work, since Server#stop can only stop a process that has been spawn by the current process. To implement killing a process spawned by a previous guard-jasmine instance, the pid must be written to a file and be read again on startup.

@mutru
Copy link
Contributor Author

mutru commented Aug 29, 2012

Feel free to close this, since we came up with a workaround. I think the issue still exists, though.

My intention was to kill only the processes spawned by the current process. To my understanding, the issue looks like this:

  • cli.rb starts Jasmine server
  • server.rb spawns the rackup process and waits until the TCP socket is listening
  • cli.rb calls runner_available?, that tries to establish an HTTP request. It times out in 15 seconds (hardcoded), because my Rails app takes too long to boot. runner_available? returns false.
  • cli.rb calls Process.exit without stopping the child process.

It looks like the child process didn't die gracefully, and I had to 'kill -9' it. The situation might look a bit different if the port was already reserved by some other process in the beginning, but it's also kind of problematic, because it's not properly handled (you could be running tests against a wrong version of code).

@netzpirat netzpirat merged commit db095a3 into guard:master Aug 29, 2012
@netzpirat
Copy link
Contributor

Oh, you understand my code better than I do, thanks for the explanation :P I've merged you're changed and also added a new :server_timeout option, so you can change the 15 seconds runner wait time.

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

Successfully merging this pull request may close these issues.

3 participants