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

Add regex filters #589

Merged
merged 7 commits into from
Jun 24, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ report.

### Defining custom filters

You can currently define a filter using either a String (that will then be Regexp-matched against each source file's path),
You can currently define a filter using either a String or Regexp (that will then be Regexp-matched against each source file's path),
a block or by passing in your own Filter class.

#### String filter
Expand All @@ -275,6 +275,16 @@ end

This simple string filter will remove all files that match "/test/" in their path.

#### Regex filter

```ruby
SimpleCov.start do
add_filter %r{^/test/}
end
```

This simple regex filter will remove all files that start with /test/ in their path.

#### Block filter

```ruby
Expand Down Expand Up @@ -305,6 +315,17 @@ Defining your own filters is pretty easy: Just inherit from SimpleCov::Filter an
the filter, a true return value from this method will result in the removal of the given source_file. The filter_argument method
is being set in the SimpleCov::Filter initialize method and thus is set to 5 in this example.

#### Array filter

```ruby
SimpleCov.start do
proc = Proc.new { |source_file| fales }
add_filter ["string", /regex/, proc, LineFilter.new(5)]
end
```

You can pass in an array containing any of the other filter types.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! 👍


#### Ignoring/skipping code

You can exclude code from the coverage report by wrapping it in `# :nocov:`.
Expand Down
10 changes: 5 additions & 5 deletions lib/simplecov/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -291,14 +291,14 @@ def add_group(group_name, filter_argument = nil, &filter_proc)
# The actual filter processor. Not meant for direct use
#
def parse_filter(filter_argument = nil, &filter_proc)
filter = filter_argument || filter_proc

if filter_argument.is_a?(SimpleCov::Filter)
filter_argument
elsif filter_proc
SimpleCov::BlockFilter.new(filter_proc)
elsif filter_argument
SimpleCov::Filter.class_for_argument(filter_argument).new(filter_argument)
elsif filter
SimpleCov::Filter.class_for_argument(filter).new(filter)
else
raise ArgumentError, "Please specify either a string or a block to filter with"
raise ArgumentError, "Please specify either a filter or a block to filter with"
end
end
end
Expand Down
20 changes: 17 additions & 3 deletions lib/simplecov/filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ def self.class_for_argument(filter_argument)
SimpleCov::RegexFilter
elsif filter_argument.is_a?(Array)
SimpleCov::ArrayFilter
elsif filter_argument.is_a?(Proc)
SimpleCov::BlockFilter
else
raise ArgumentError, "You have provided an unrecognized filter type"
end
Expand Down Expand Up @@ -63,11 +65,23 @@ def matches?(source_file)
end

class ArrayFilter < SimpleCov::Filter
# Returns true if any of the file paths passed in the given array matches the string
# configured when initializing this Filter with StringFilter.new(['some/path', 'other/path'])
def initialize(filter_argument)
filter_argument.map! do |arg|
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should rather not use map! as I don't wanna mutate whatever was passed in, so rather without ! :)

Copy link
Author

Choose a reason for hiding this comment

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

Good call.

if arg.is_a?(SimpleCov::Filter)
arg
else
Filter.class_for_argument(arg).new(arg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

just thinking out loud, couldn't we also put that check into Filter.class_for_argument e.g. if it is already a filter? Would make that code easier and centralize the knowledge :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah wait we can't because this one already instantiates which we wouldn't need there... maybe another helper method? Not quite sure.

Copy link
Author

Choose a reason for hiding this comment

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

Yah sounds good. I'll make a factory function that instantiates based the appropriate class type, or does nothing if it's given a filter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

didn't we wanna use .build_filter here? :)

Copy link
Author

Choose a reason for hiding this comment

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

Right thanks. I knew there was a reason I abstracted that out.

end
end

super(filter_argument)
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a feeling, the additions here were necessary because THINGS weren't working. Sounds like we might miss some specs here. If you wanna add those, great! 👍

If not let me know and I'll try to get to it when I got some time :)

Copy link
Author

Choose a reason for hiding this comment

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

There is definitely a hole in the specs. I had to do manual tests to make sure this worked. I'll see if I can whip up some specs.


# Returns true if any of the filters in the array match the given source file.
# Configure this Filter like StringFilter.new(['some/path', /^some_regex/, Proc.new {|src_file| ... }])
def matches?(source_files_list)
filter_argument.any? do |arg|
source_files_list.project_filename =~ /#{arg}/
arg.matches?(source_files_list)
end
end
end
Expand Down