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

Default code path doesn't appear to handle fork correctly? #371

Closed
ioquatix opened this issue Feb 10, 2015 · 8 comments · Fixed by #377
Closed

Default code path doesn't appear to handle fork correctly? #371

ioquatix opened this issue Feb 10, 2015 · 8 comments · Fixed by #377

Comments

@ioquatix
Copy link

I had to do this.

https://gist.github.com/clicube/5017378

This removed what appeared to be superfluous output:

Coverage report generated for RSpec to /home/samuel/Documents/Programming/rubydns/coverage. 0 / 599 LOC (0.0%) covered.

multiple times per run.

Perhaps this should be the default behaviour?

@bf4
Copy link
Collaborator

bf4 commented Feb 18, 2015

pid = Process.pid
SimpleCov.at_exit do
  SimpleCov.result.format! if Process.pid == pid
end

I like that.

@bf4
Copy link
Collaborator

bf4 commented Feb 18, 2015

are you using a merge timeout?

@ioquatix
Copy link
Author

What do you mean?

Sent from my phone.

On 18/02/2015, at 2:28 pm, Benjamin Fleischer notifications@github.com wrote:

are you using a merge timeout?


Reply to this email directly or view it on GitHub.

@bf4
Copy link
Collaborator

bf4 commented Feb 18, 2015

Would you paste your simplecov config? (How you require, configure, start it)

@coderanger
Copy link
Contributor

My solution for this was to add SimpleCov.at_exit { SimpleCov.instance_variable_set(:@result, nil) } to the post-fork code. Might be nice if the main at_exit implementation checked pids and only ran formatters or the minimum coverage check if the current pid was the one you ran start in.

Code that forks. SimpleCov start.

@bf4
Copy link
Collaborator

bf4 commented Mar 11, 2015

I'm up for a PR

@coderanger
Copy link
Contributor

Mostly trying to think if that would break anything. The Coverage module already doesn't actually work across a fork, so I think it should be safe in all cases?

@colszowka
Copy link
Collaborator

Yes, I think that would be a good change - although maybe we can wrap the top level pid under SimpleCov.pid so we don't get weird interferences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants