more efficient simplecov filters

Author: grodowski

lib/undercover/filter_set.rb

@@ -2,22 +2,36 @@
 
 module Undercover
   class FilterSet
-    attr_reader :allow_filters, :reject_filters, :simplecov_ignored_files
+    attr_reader :allow_filters, :reject_filters, :simplecov_filters
 
-    def initialize(allow_filters, reject_filters, simplecov_ignored_files)
+    def initialize(allow_filters, reject_filters, simplecov_filters)
       @allow_filters = allow_filters || []
       @reject_filters = reject_filters || []
-      @simplecov_ignored_files = simplecov_ignored_files
+      @simplecov_filters = simplecov_filters || []
     end
 
     def include?(filepath)
       fnmatch = proc { |glob| File.fnmatch(glob, filepath, File::FNM_EXTGLOB) }
 
       # Check if file was ignored by SimpleCov filters
-      return false if simplecov_ignored_files.include?(filepath)
+      return false if ignored_by_simplecov?(filepath)
 
       # Apply Undercover's own filters
       allow_filters.any?(fnmatch) && reject_filters.none?(fnmatch)
     end
+
+    private
+
+    def ignored_by_simplecov?(filepath)
+      simplecov_filters.any? do |filter|
+        if filter[:string]
+          filepath.include?(filter[:string])
+        elsif filter[:regex]
+          filepath.match?(Regexp.new(filter[:regex]))
+        elsif filter[:file]
+          filepath == filter[:file]
+        end
+      end
+    end
   end
 end

lib/undercover/simplecov_formatter.rb

@@ -15,15 +15,46 @@ def export_path
 
 module SimpleCov
   class << self
-    attr_accessor :filtered_files
+    attr_accessor :filter_definitions
 
     alias filtered_uncached filtered
 
     def filtered(files)
-      @filtered_files ||= Set.new
+      @filter_definitions ||= extract_filter_definitions
       original_files = files.dup
       filtered_uncached(files).tap do |filtered_files|
-        @filtered_files += (original_files.map(&:filename) - filtered_files.map(&:filename))
+        filtered_file_paths = (original_files.map(&:filename) - filtered_files.map(&:filename))
+        filtered_file_paths.each do |file|
+          relative_path = file.delete_prefix("#{SimpleCov.root}/")
+          @filter_definitions << {file: relative_path} unless covered_by_serializable_filters?(relative_path)
+        end
+      end
+    end
+
+    private
+
+    def extract_filter_definitions
+      filter_array = []
+
+      filters.each do |filter|
+        case filter
+        when SimpleCov::StringFilter
+          filter_array << {string: filter.filter_argument}
+        when SimpleCov::RegexFilter
+          filter_array << {regex: filter.filter_argument.source}
+        end
+      end
+
+      filter_array
+    end
+
+    def covered_by_serializable_filters?(relative_path)
+      @filter_definitions.any? do |filter_def|
+        if filter_def[:string]
+          relative_path.include?(filter_def[:string])
+        elsif filter_def[:regex]
+          relative_path.match?(Regexp.new(filter_def[:regex]))
+        end
       end
     end
   end
@@ -48,11 +79,7 @@ def add_undercover_meta_fields
     end
 
     def add_ignored_files
-      ignored_files = SimpleCov.filtered_files&.map do |file|
-        file.delete_prefix("#{SimpleCov.root}/")
-      end || []
-
-      formatted_result[:meta][:ignored_files] = ignored_files
+      formatted_result[:meta][:ignored_files] = SimpleCov.filter_definitions || []
     end
 
     # format_files uses relative path as keys, as opposed to the superclass method

spec/cli_spec.rb

@@ -274,7 +274,7 @@
 
     json_content = {
       'meta' => {
-        'ignored_files' => ['app/lib/temp/temp_file.rb', 'db/migrate/migration.rb']
+        'ignored_files' => [{'string' => 'app/lib/temp/'}, {'file' => 'db/migrate/migration.rb'}]
       },
       'coverage' => {
         'app/models/user.rb' => {'lines' => [1, 0]}
@@ -289,7 +289,8 @@
 
     simplecov_adapter = double('SimpleCov adapter',
                                coverage: [],
-                               ignored_files: ['app/lib/temp/temp_file.rb', 'db/migrate/migration.rb'])
+                               ignored_files: [{'string' => 'app/lib/temp/'},
+                                               {'file' => 'db/migrate/migration.rb'}])
     allow(Undercover::SimplecovResultAdapter).to receive(:parse).and_return(simplecov_adapter)
 
     allow_any_instance_of(Undercover::Report).to receive(:validate) { nil }
@@ -299,7 +300,7 @@
     expect(Undercover::FilterSet).to receive(:new).with(
       ['*.rb', '*.rake', '*.ru', 'Rakefile'],
       ['test/*', 'spec/*', 'db/*', 'config/*', '*_test.rb', '*_spec.rb'],
-      ['app/lib/temp/temp_file.rb', 'db/migrate/migration.rb']
+      [{'string' => 'app/lib/temp/'}, {'file' => 'db/migrate/migration.rb'}]
     ).once.and_call_original
 
     subject.run(['-s', 'test.json'])

spec/filter_set_spec.rb

@@ -6,9 +6,9 @@
 describe Undercover::FilterSet do
   let(:allow_filters) { ['*.rb'] }
   let(:reject_filters) { ['*_spec.rb'] }
-  let(:simplecov_ignored_files) { ['app/lib/filtered_file.rb'] }
+  let(:simplecov_filters) { [{file: 'app/lib/filtered_file.rb'}] }
 
-  subject(:filter_set) { described_class.new(allow_filters, reject_filters, simplecov_ignored_files) }
+  subject(:filter_set) { described_class.new(allow_filters, reject_filters, simplecov_filters) }
 
   describe '#include?' do
     context 'when file is in SimpleCov ignored files' do
@@ -32,7 +32,7 @@
     end
 
     context 'with empty SimpleCov ignored files' do
-      let(:simplecov_ignored_files) { [] }
+      let(:simplecov_filters) { [] }
 
       it 'behaves like the original FilterSet' do
         expect(filter_set.include?('app/models/user.rb')).to be true
@@ -44,7 +44,7 @@
     context 'with complex glob patterns' do
       let(:allow_filters) { ['*.rb', '*.rake', 'Rakefile'] }
       let(:reject_filters) { ['test/*', 'spec/*'] }
-      let(:simplecov_ignored_files) { ['lib/migrations/20230101_create_users.rb'] }
+      let(:simplecov_filters) { [{file: 'lib/migrations/20230101_create_users.rb'}] }
 
       it 'correctly applies all filters' do
         expect(filter_set.include?('app/models/user.rb')).to be true
@@ -54,5 +54,76 @@
         expect(filter_set.include?('lib/migrations/20230101_create_users.rb')).to be false
       end
     end
+
+    context 'with string and regex filters' do
+      let(:simplecov_filters) do
+        [
+          {string: 'spec/'},
+          {regex: '\/test\/'},
+          {file: 'custom_ignored.rb'},
+        ]
+      end
+
+      it 'correctly evaluates string filters' do
+        expect(filter_set.include?('spec/user_spec.rb')).to be false
+        expect(filter_set.include?('app/spec/helper.rb')).to be false
+      end
+
+      it 'correctly evaluates regex filters' do
+        expect(filter_set.include?('app/test/unit_test.rb')).to be false
+        expect(filter_set.include?('lib/test/integration_test.rb')).to be false
+      end
+
+      it 'correctly evaluates file filters' do
+        expect(filter_set.include?('custom_ignored.rb')).to be false
+      end
+
+      it 'allows files not matching any filter' do
+        expect(filter_set.include?('app/models/user.rb')).to be true
+      end
+
+      it 'handles file filter that does not match' do
+        expect(filter_set.include?('different_file.rb')).to be true
+      end
+
+      it 'handles file filter that returns false when filepath does not match exactly' do
+        file_filter_set = described_class.new(['*.rb'], [], [{file: 'exact_match.rb'}])
+        expect(file_filter_set.include?('different_file.rb')).to be true
+        expect(file_filter_set.include?('exact_match.rb')).to be false
+      end
+
+      it 'explicitly tests file filter branch where comparison returns false' do
+        test_filter_set = described_class.new(['*.rb'], [], [{file: 'specific_file.rb'}])
+        expect(test_filter_set.include?('other_file.rb')).to be true
+      end
+
+      it 'tests the false branch of file filter comparison within any loop' do
+        multi_filter_set = described_class.new(['*.rb'], [], [
+                                                 {file: 'will_not_match.rb'},
+                                                 {string: 'also_will_not_match'},
+                                               ])
+        expect(multi_filter_set.include?('some_other_file.rb')).to be true
+      end
+
+      it 'specifically tests file filter false return in isolation' do
+        isolated_filter_set = described_class.new(['*.rb'], [], [{file: 'exact_name.rb'}])
+        expect(isolated_filter_set.include?('totally_different.rb')).to be true
+        expect(isolated_filter_set.include?('exact_name.rb')).to be false
+      end
+
+      it 'forces file filter false evaluation by using non-matching filename' do
+        force_false_set = described_class.new(['*.rb'], [], [{file: 'specific_file.rb'}])
+        expect(force_false_set.include?('different_file.rb')).to be true
+      end
+
+      it 'tests the elsif branch condition itself with falsy file value' do
+        falsy_filter_set = described_class.new(['*.rb'], [], [
+                                                 {file: nil},
+                                                 {file: ''},
+                                                 {string: 'will_not_match'},
+                                               ])
+        expect(falsy_filter_set.include?('any_file.rb')).to be true
+      end
+    end
   end
 end

spec/fixtures/simplecov_with_ignored_files.json

@@ -3,9 +3,9 @@
     "timestamp": 1579085826,
     "simplecov_root": "/Users/mrgrodo/dev/undercover",
     "ignored_files": [
-      "app/lib/temp/temp_file.rb",
-      "db/migrate/20230101_create_users.rb",
-      "test/factories/user_factory.rb"
+      { "string": "app/lib/temp/" },
+      { "regex": "\/migrate\/" },
+      { "file": "test/factories/user_factory.rb" }
     ]
   },
   "coverage": {

spec/report_spec.rb

@@ -240,7 +240,7 @@
     end
 
     it 'creates FilterSet with empty ignored files' do
-      expect(report.filter_set.simplecov_ignored_files).to eq([])
+      expect(report.filter_set.simplecov_filters).to eq([])
     end
 
     it 'behaves like the original implementation' do