Add subprocess handling to simplecov #881

robotdana · 2020-03-03T15:59:05Z

Using Process.fork (whether using the Parallel gem or directly)
creates code that was invisible to SimpleCov. by starting SimpleCov
within the subprocess with its own command name and etc we can
see that code :)

This also adds documentation for what to do when using Process.spawn or
similar.

fixes: #414 and probably others

This seems the more correct place to fix this than my previous attempt here: grosser/parallel#275

Testing was a bit of a mess, perhaps you have some ideas,
Also what are your thoughts on this being enabled by default or not?

Thanks :)

Using Process.fork (whether using the Parallel gem or directly) creates code that was invisible to SimpleCov. by starting SimpleCov within the subprocess with its own command name and etc we can see that code :) This also adds documentation for what to do when using Process.spawn or similar. fixes: simplecov-ruby#414

robotdana · 2020-03-03T16:05:13Z

(I'm not clear why test 2.4.9 didn't work, it doesn't seem related?)

PragTob

Hi there and thank you very much for your contribution! 👍 💚 🌟

Seems like a reasonably simple solution to a hard problem.

I'm pondering the freedom patch at the moment, first if we should use it by default and secondly I'm thinking a bit about whether we should have it at all. Reason being while it gives us flexibility, I try to only do it in the most extreme of cases. Plus, it's another integration to maintain. So, my thinking is: Should we ship this as part of SimpleCov or is the excellent (! 💚 !) documentation enough so that people could just add code to their .fork calls themselves? Or even add the freedom patch themselves?

Just something I'm pondering right now/am undecided about.

For testing: You're right - it's a bit odd since it relies on global state. The best testing strategy I could come up with is to do it in the feature tests either with a new test project for testing sub processes or adding it to one of the existing test projects. That way, the global nature of what happens is isolated to the simulated test run we do. At the same time it shows on a full project scale that this works. I know they're more complicated to implement but I'm happy to point you in the right direction.

Again, thank you so much for this PR - let's see how we can get this merged!

README.md

lib/simplecov.rb

lib/simplecov/configuration.rb

spec/simplecov_spec.rb

PragTob · 2020-03-03T17:46:34Z

Ah forgot, I'm also not sure why the test run failed. I'll save it's error message here and rerun it.

336 examples, 0 failures

Randomized with seed 39135

	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:388:in `process_coverage_result'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:108:in `result'
	from /home/runner/work/simplecov/simplecov/lib/simplecov/configuration.rb:196:in `block in at_exit'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:215:in `run_exit_tasks!'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:203:in `at_exit_behavior'
	from /home/runner/work/simplecov/simplecov/lib/simplecov/defaults.rb:27:in `block in <top (required)>'
/home/runner/.rubies/ruby-2.4.9/bin/ruby -I/home/runner/.rubies/ruby-2.4.9/lib/ruby/gems/2.4.0/gems/rspec-core-3.9.1/lib:/home/runner/.rubies/ruby-2.4.9/lib/ruby/gems/2.4.0/gems/rspec-support-3.9.2/lib /home/runner/.rubies/ruby-2.4.9/lib/ruby/gems/2.4.0/gems/rspec-core-3.9.1/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb failed
##[error]Process completed with exit code 1.

PragTob · 2020-03-03T17:50:18Z

Failure definitely seems related, now it's happening in 2.6.5 - I guess it's some kind of race condition introduced by the test calling .result - moving it to cukes would also get rid off that :)

 /home/runner/.rubies/ruby-2.6.5/bin/ruby -I/home/runner/.rubies/ruby-2.6.5/lib/ruby/gems/2.6.0/gems/rspec-core-3.9.1/lib:/home/runner/.rubies/ruby-2.6.5/lib/ruby/gems/2.6.0/gems/rspec-support-3.9.2/lib /home/runner/.rubies/ruby-2.6.5/lib/ruby/gems/2.6.0/gems/rspec-core-3.9.1/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb

Randomized with seed 17877
/home/runner/work/simplecov/simplecov/lib/simplecov.rb:399:in `result': coverage measurement is not enabled (RuntimeError)
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:399:in `adapt_coverage_result'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:388:in `process_coverage_result'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:108:in `result'
	from /home/runner/work/simplecov/simplecov/lib/simplecov/configuration.rb:196:in `block in at_exit'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:215:in `run_exit_tasks!'
	from /home/runner/work/simplecov/simplecov/lib/simplecov.rb:203:in `at_exit_behavior'
	from /home/runner/work/simplecov/simplecov/lib/simplecov/defaults.rb:27:in `block in <top (required)>'

Can be enabled with enable_for_subprocesses Also moved the testing to a feature test

robotdana · 2020-03-03T21:24:30Z

The new failing cuke, so 2.4 is certain it's looked at 2 files, and 2.7 is certain it's looked at 1 file, without changing anything else, which is not something i was expecting. it passes on 2.7, it passes on 2.4 if i change a 1 to a 2. I'm gonna figure that part out later today. happier with the config and cuke test otherwise?

robotdana · 2020-03-04T01:00:04Z

reminder to myself: I can probably solve the file count issues with a filter

robotdana · 2020-03-04T05:39:07Z

So its failing for jruby is interesting. fork isn't available. is it even relevant? should we just suppress running that cuke on jruby 9? I have never used jruby before so i'm not even sure what goes into running it locally

robotdana · 2020-03-06T04:20:58Z

Do you have a preference for how to handle the jruby issue?

PragTob

Hey,

sorry for the delay I was a bit busy preparing a talk and stuff 😅

Looks good, I left some small in line comments.

With regards to JRuby yes, if memory serves right JRuby doesn't support Process forking. Skipping the tests for JRuby is the right call.

In env.rb we already skip tests if branch coverage isn't supported:

Before("@branch_coverage") do
  skip_this_scenario unless SimpleCov.branch_coverage_supported?
end

So tagging the feature/secnario with @fork and doing the same skipping for JRuby. Turns out we already have a minimal forking spec in the faked_project where we also skip on JRuby

Hope that helps!

Cheers and thanks for all your work! 💚

lib/simplecov/configuration.rb

test_projects/subprocesses/.simplecov

Also Gem::Version.new comparisons please. Also don't run this test on jruby, Process.fork is NotImplementedError

robotdana · 2020-03-06T10:46:34Z

Thank you for the rabbits ❤️ 🐰

PragTob

Looking great, thank you very much!

@robotdana

Update ruby-simplecov to 0.19.0. 0.19.0 (2020-08-16) ========== ## Breaking Changes * Dropped support for Ruby 2.4, it reached EOL ## Enhancements * observe forked processes (enable with SimpleCov.enable_for_subprocesses). See [#881](simplecov-ruby/simplecov#881), thanks to [@robotdana](https://github.com/robotdana) * SimpleCov distinguishes better that it stopped processing because of a previous error vs. SimpleCov is the originator of said error due to coverage requirements. ## Bugfixes * Changing the `SimpleCov.root` combined with the root filtering didn't work. Now they do! Thanks to [@deivid-rodriguez](https://github.com/deivid-rodriguez) and see [#894](simplecov-ruby/simplecov#894) * in parallel test execution it could happen that the last coverage result was written to disk when it didn't complete yet, changed to only write it once it's the final result * if you run parallel tests only the final process will report violations of the configured test coverage, not all previous processes * changed the parallel_tests merging mechanisms to do the waiting always in the last process, should reduce race conditions ## Noteworthy * The repo has moved to https://github.com/simplecov-ruby/simplecov - everything stays the same, redirects should work but you might wanna update anyhow * The primary development branch is now `main`, not `master` anymore. If you get simplecov directly from github change your reference. For a while `master` will still be occasionally updated but that's no long term solion.

PragTob reviewed Mar 3, 2020

View reviewed changes

Subprocesses is off by default

33084cc

Can be enabled with enable_for_subprocesses Also moved the testing to a feature test

Fix test to work for ruby 2.4 also

c6a65c6

Have spawn doc match closer to spec

d59fd44

PragTob reviewed Mar 6, 2020

View reviewed changes

lib/simplecov/configuration.rb Outdated Show resolved Hide resolved

lib/simplecov/configuration.rb Outdated Show resolved Hide resolved

test_projects/subprocesses/.simplecov Show resolved Hide resolved

Remove unnecessary =

fa0f73d

Also Gem::Version.new comparisons please. Also don't run this test on jruby, Process.fork is NotImplementedError

PragTob approved these changes Mar 7, 2020

View reviewed changes

PragTob merged commit 94eca16 into simplecov-ruby:master Mar 7, 2020

robotdana mentioned this pull request Apr 25, 2020

Adding a hook for having just started a process? (making SimpleCov work) grosser/parallel#275

Closed

tagliala mentioned this pull request Sep 7, 2020

warning: instance variable @enable_for_subprocesses not initialized #926

Closed

dependabot bot mentioned this pull request Mar 11, 2021

Bump simplecov from 0.18.5 to 0.21.2 tungnt247/socify#39

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subprocess handling to simplecov #881

Add subprocess handling to simplecov #881

robotdana commented Mar 3, 2020

robotdana commented Mar 3, 2020

PragTob left a comment

PragTob commented Mar 3, 2020

PragTob commented Mar 3, 2020

robotdana commented Mar 3, 2020

robotdana commented Mar 4, 2020

robotdana commented Mar 4, 2020 •

edited

Loading

robotdana commented Mar 6, 2020

PragTob left a comment

robotdana commented Mar 6, 2020

PragTob left a comment

Add subprocess handling to simplecov #881

Add subprocess handling to simplecov #881

Conversation

robotdana commented Mar 3, 2020

robotdana commented Mar 3, 2020

PragTob left a comment

Choose a reason for hiding this comment

PragTob commented Mar 3, 2020

PragTob commented Mar 3, 2020

robotdana commented Mar 3, 2020

robotdana commented Mar 4, 2020

robotdana commented Mar 4, 2020 • edited Loading

robotdana commented Mar 6, 2020

PragTob left a comment

Choose a reason for hiding this comment

robotdana commented Mar 6, 2020

PragTob left a comment

Choose a reason for hiding this comment

robotdana commented Mar 4, 2020 •

edited

Loading