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

Fix regexp for filtering out non-root results being set too early #894

Merged
merged 1 commit into from Jun 21, 2020

Conversation

deivid-rodriguez
Copy link
Collaborator

If user configures a custom SimpleCov.root, results outside of the new root wouldn't be properly filtered out since the regexp for filtering was being set at require time, before the SimpleCov.root configuration had been applied.

Instead, lazily set this regexp, so that by the time it is used, the new SimpleCov.root value is already setup.

Fixes #885.

Copy link
Collaborator

@PragTob PragTob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix 💚 and good eyes 👀 👀 👀

Minor spec comment but overall happy with merge 🎉

IMG_20171225_092009_Bokeh

spec/useless_results_remover_spec.rb Outdated Show resolved Hide resolved
@deivid-rodriguez deivid-rodriguez force-pushed the lazily_set_root_regexp branch 3 times, most recently from 7c36827 to 4371596 Compare June 21, 2020 09:35
Copy link
Collaborator

@PragTob PragTob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👌

IMG_20200127_133956

@PragTob
Copy link
Collaborator

PragTob commented Jun 21, 2020

Doesn't look good to CI though:

/home/runner/.rubies/ruby-2.4.10/lib/ruby/gems/2.4.0/gems/bundler-2.1.4/lib/bundler/spec_set.rb:86:in `block in materialize': Could not find rspec-support-3.9.3 in any of the sources (Bundler::GemNotFound)
	from /home/runner/.rubies/ruby-2.4.10/lib/ruby/gems/2.4.0/gems/bundler-2.1.4/lib/bundler

If user configures a custom `SimpleCov.root`, results outside of the new
root wouldn't be properly filtered out since the regexp for filtering
was being set at require time, before the `SimpleCov.root` configuration
had been applied.

Instead, lazily set this regexp, so that by the time it is used, the new
`SimpleCov.root` value is already setup.
@PragTob PragTob merged commit 72317c9 into master Jun 21, 2020
@PragTob PragTob deleted the lazily_set_root_regexp branch June 21, 2020 10:16
PragTob added a commit that referenced this pull request Jun 21, 2020
@mrcasals
Copy link

Yay! Thank you so much for taking care of this ❤️

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 14, 2020
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.
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.

Multiple engines requiring each other only show their own files in the reports
3 participants