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

Apply simplecov filters #38

Merged
merged 2 commits into from
Dec 1, 2018
Merged

Apply simplecov filters #38

merged 2 commits into from
Dec 1, 2018

Conversation

tomeon
Copy link
Collaborator

@tomeon tomeon commented Oct 22, 2018

This changeset introduces a new flag, --prefilter, which causes Bashcov to omit from the coverage results any files that match one or more of the filters in SimpleCov.filters.

The motivation for this feature is that SimpleCov itself will ignore these files, so Bashcov is doing work (e.g., detecting whether a file is a shell script) only for that work to be thrown away. This is especially problematic when Bashcov.root_directory sits at the top of a large directory hierarchy; e.g., a project that includes vendored dependencies.

I've made this behavior optional, in case there are any projects in the wild that depend on Bashcov's current approach to generating the coverage results hash. However, I tend to think that this behavior should be the default. Thoughts, @infertux?

@tomeon
Copy link
Collaborator Author

tomeon commented Oct 22, 2018

Also -- I wonder if it is worth considering whether to subclass SimpleCov. This would make it possible to do things like override the use of SimpleCov::LinesClassifier, replacing it with the use of Bashcov::Lexer. It's possible that this would cut down on the amount of work Bashcov::Runner has to do to prepare the coverage results for handoff to Simplecov.

@coveralls
Copy link

coveralls commented Oct 22, 2018

Coverage Status

Coverage increased (+0.04%) to 99.054% when pulling deec72e on apply-simplecov-filters into a4d15bd on master.

@infertux
Copy link
Owner

I've made this behavior optional, in case there are any projects in the wild that depend on Bashcov's current approach to generating the coverage results hash. However, I tend to think that this behavior should be the default. Thoughts, @infertux?

I agree this should be the default. I'm tempted to remove the --prefilter flag altogether and bump the major version to 2.0.0 as per semantic versioning. That way, if any projects in the wild depends on the current behavior, they will know the new version isn't necessarily backward compatible. What do you reckon?

I wonder if it is worth considering whether to subclass SimpleCov. This would make it possible to do things like override the use of SimpleCov::LinesClassifier, replacing it with the use of Bashcov::Lexer. It's possible that this would cut down on the amount of work Bashcov::Runner has to do to prepare the coverage results for handoff to Simplecov.

Sounds like a great idea to me. However I'm too busy with other projects at the moment to spend any time on it. Maybe open a new issue to keep track of it :)

Copy link
Owner

@infertux infertux left a comment

Choose a reason for hiding this comment

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

from coverage results by applying SimpleCov filters to the list of
"tracked" files.

Also ensure that Bashcov always includes files matched by
SimpleCov.tracked_files in the coverage results.
by (1) making certain examples ignore uncovered files and (2) using
./spec/test_app as the Bashcov.root_directory for certain other
examples
@tomeon
Copy link
Collaborator Author

tomeon commented Nov 25, 2018

I'm tempted to remove the --prefilter flag altogether and bump the major version to 2.0.0 as per semantic versioning.

That seems 100% reasonable to me :) -- I've reworked the PR to remove the --prefilter flag and corresponding code in Bashcov and Bashcov::Runner.

As for subclassing SimpleCov -- turns out that (strictly speaking) it can't be done, since SimpleCov is a module. I fooled around with other approaches, including doing refinements on SimpleCov.singleton_class, but nothing other than outright monkeypatching seems viable. If and when I have a substantial chunk of free time, I may open a PR over at the SimpleCov repo that permits plugging in custom LinesClassifer-type classes/objects (and maybe introduces other points of customization).

@infertux infertux merged commit bc03077 into master Dec 1, 2018
@infertux infertux deleted the apply-simplecov-filters branch December 1, 2018 06:13
@infertux
Copy link
Owner

infertux commented Dec 1, 2018

Awesome @tomeon - I gave it a quick test run and couldn't find any issues.

While we're making potentially breaking changes, I'd like to wait for Ruby 2.6 to roll out before I cut the 2.0.0 release and drop support for Ruby 2.3. Let me know if you want me to push your changes to RubyGems ASAP and I'll push a prerelease in the meantime.

As always, your work on Bashcov is much appreciated, thank you :)

infertux added a commit that referenced this pull request Dec 1, 2018
@tomeon
Copy link
Collaborator Author

tomeon commented Dec 1, 2018

@infertux -- thank you very much! No rush for a pre-release; the project that I need this change for uses a Gemfile, so I can peg the Bashcov dep to the relevant commit until 2.0.0 is available.

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

Successfully merging this pull request may close these issues.

None yet

3 participants