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

Use at_exit instead of overriding trap for interrupt to shutdown ProxyServer, AuthenticatedProxyServer #176

Closed
wants to merge 5 commits into from

Conversation

olegkovalenko
Copy link

Call to trap replaces handler for specified signal,
so ProxyServer.shutdown would have never been called.
at_exit registers handler for execution when the programm
exits, handlers are executed in revers order of registration.

…Server, AuthenticatedProxyServer.

Call to trap replaces handler for specified signal,
so ProxyServer.shutdown would have never been called.
at_exit registers handler for execution when the programm
exits, handlers are executed in revers order of registration.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 06af507 on olegkovalenko:master into 24428d2 on httprb:master.

@tarcieri
Copy link
Member

Hmm, I think the best solution here is to factor ProxyServer startup/shutdown into before/after blocks...

@ixti
Copy link
Member

ixti commented Jan 18, 2015

Although this patch really makes things a bit better, it doesn't fixes same issue with example_server, and I agree with @tarcieri it's better to start/stop it with before/after blocks.

@ixti
Copy link
Member

ixti commented Jan 19, 2015

I have re-factored how we start ExampleServer (also renamed it to DummyServer) in specs.
You can check new implementation and kind of follow same way if you want, or I will jump on this later.

@ixti
Copy link
Member

ixti commented Jan 19, 2015

Ouch. My last commit broke your last commit :D

Please, don't put before blocks into spec_helper.
Instead add RSpec.configure do |config| to the end of proxy_server with your code.
And remove ExampleServer start out of there - I have completely refactored it.

* upstream/master:
  Require proxy_server ONLY in spec that needs it
  Refactor ExampleServer to use before/after hook

Conflicts:
	spec/support/example_server.rb
@olegkovalenko
Copy link
Author

No worries ;)

@tarcieri
Copy link
Member

Typically I start and stop these sorts of things in conjunction with the tests that depend on them, rather than globally in the suite.

@ixti ixti closed this in 0c8f268 Jan 20, 2015
@ixti
Copy link
Member

ixti commented Jan 20, 2015

Thanks for your PR, but I have implemented it in a bit different way :D
In any case thank you very much.

@olegkovalenko
Copy link
Author

(handshake)
Thanks for your time and suggestions ;)

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.

4 participants