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

capybara's finalization process is not working with test-queue #83

Open
mtakahashi-mst opened this issue Jun 26, 2018 · 7 comments
Open

Comments

@mtakahashi-mst
Copy link

test-queue exits worker process by calling Kernel#exit!. Kernel#exit! won't call finalization handler of a process registered by Kernel.at_exit.
On the other hand, capybara's finalization process is registered by Kernel.at_exit, so it won't be called when a worker process ends.

How can I finalize Selenium driver?
Should I override TestQueue::Runnder::spawn_workers and change Kernel#exit! to Kernel#exit ?

@garybernhardt
Copy link
Collaborator

We're seeing the same behavior, also using Selenium via Capybara. Each test run leaks chromedriver processes, which leads the machine to run out of file descriptors pretty quickly.

Narrowing it down:

  • The processes aren't leaked when running vanilla rspec, or when running via parallel_rspec, so this seems specific to test-queue.
  • Changing all exit!s in lib/test_queue/runner.rb to vanilla exits stops processes from leaking, presumably for the reason @mtakahashi-mst described. Everything else seems to continue working with that change.

Presumably there's a reason that test-queue calls exit! and not exit, but I don't see it written down and nothing comes to mind. Any thoughts from folks who are more familiar with the code?

@tmm1
Copy link
Owner

tmm1 commented Jun 28, 2018

IIRC, test-unit and other libs use at_exit hooks to trigger their own reporting which didn't make any sense when running inside test-queue.

garybernhardt added a commit to garybernhardt/test-queue that referenced this issue Jun 28, 2018
This is a short-term fix to allow us to override `exit!` in subclasses
of Runner, replacing it with a call to `exit`, which allows at_exit
handlers to run, which allows test processes to clean up after
themselves properly, which allows Capybara to do finalization and avoid
leaking Selenium processes (as reported in
tmm1#83).
@garybernhardt
Copy link
Collaborator

@mtakahashi-mst, I made a change that fixes this for us. Can you give it a try? It's in my fork; you can add it to your Gemfile with:

  gem 'test-queue', git: 'https://github.com/garybernhardt/test-queue.git', ref: '5119608'

(If anyone else is lurking on this issue, please let us know whether that works for you. It's a simple change but we need to know whether there are any weird side effects.)

@mtakahashi-mst
Copy link
Author

mtakahashi-mst commented Jul 11, 2018

@garybernhardt
I overrides Runner.spawn_workers. This is OK for my case.

If you change as a gem, your changes are unbalanced.

  • exit in Runner.exeute becomes to throw SystemExit Exception. Runner.exeute is called in user's application context.
  • (This is not your change but) Runner.abort kills sub process by SIGKILL. In this case worker's finalization process not called.

@mtakahashi-mst
Copy link
Author

I prefer to change exit! only in spawn_workers to exit_worker

@garybernhardt
Copy link
Collaborator

I see what you mean. The user's application might rescue the SystemExit, which would be bad.

Adding exit_worker lets users override it easily. But users who don't override will still have this problem. (No finalizing on Selenium, or any other process.) Is that right?

@garybernhardt
Copy link
Collaborator

I'm not sure what to do here, but if @mtakahashi-mst or anyone else wants to take a crack at it, I'm happy to test for whether it works for us too!

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

No branches or pull requests

3 participants