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

Reduce memory allocated when rendering the index page #559

Merged
merged 1 commit into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 2 additions & 1 deletion lib/coverband/collectors/abstract_tracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def newly_seen_key?(key)
end

def track_key?(key, options = {})
@ignore_patterns.none? { |pattern| key.to_s.include?(pattern) }
key = key.to_s
@ignore_patterns.none? { |pattern| key.match?(pattern) }
end

private
Expand Down
6 changes: 3 additions & 3 deletions lib/coverband/collectors/view_tracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ def unused_keys(used_views = nil)
recently_used_views = used_keys.keys
unused_views = all_keys - recently_used_views
# since layouts don't include format we count them used if they match with ANY formats
unused_views.reject { |view| view.match(/\/layouts\//) && recently_used_views.any? { |used_view| view.include?(used_view) } }
unused_views.reject { |view| @ignore_patterns.any? { |pattern| view.include?(pattern) } }
unused_views.reject { |view| view.include?("/layouts/") && recently_used_views.any? { |used_view| view.include?(used_view) } }
unused_views.reject { |view| @ignore_patterns.any? { |pattern| view.match?(pattern) } }
end

def clear_key!(filename)
Expand All @@ -100,7 +100,7 @@ def clear_key!(filename)

def track_file?(file, options = {})
(file.start_with?(@project_directory) || options[:layout]) &&
@ignore_patterns.none? { |pattern| file.include?(pattern) }
@ignore_patterns.none? { |pattern| file.match?(pattern) }
end

def concrete_target
Expand Down
6 changes: 3 additions & 3 deletions lib/coverband/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def initialize
def reset
@root = Dir.pwd
@root_paths = []
@ignore = IGNORE_DEFAULTS.dup
@ignore = IGNORE_DEFAULTS.map { |ignore_str| Regexp.new(ignore_str) }
@search_paths = TRACKED_DEFAULT_PATHS.dup
@verbose = false
@reporter = "scov"
Expand Down Expand Up @@ -222,8 +222,8 @@ def search_paths=(path_array)
# Don't allow the ignore to override things like gem tracking
###
def ignore=(ignored_array)
ignored_array.map { |ignore_str| Regexp.new(ignore_str) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here was an attempt to convert keys to regexps, but because of the bug, this line was a no-op.

@ignore = (@ignore + ignored_array).uniq
ignored_array = ignored_array.map { |ignore_str| Regexp.new(ignore_str) }
@ignore |= ignored_array
Copy link
Owner

Choose a reason for hiding this comment

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

so I think we want to assign here with a uniq... the purpose of this is that folks can add overrides to ignores... traditionally those have always been strings... so it is good to fix the bug, but I think we still need the (@ignore + ignored_array).uniq type logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ignore |= ignored_array is the same as @ignore = (@ignore + ignored_array).uniq.
Correct me if I am wrong.

Copy link
Owner

Choose a reason for hiding this comment

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

ha you are correct, I just don't use the single |= very often... this works

rescue RegexpError
logger.error "an invalid regular expression was passed in, ensure string are valid regex patterns #{ignored_array.join(",")}"
end
Expand Down
4 changes: 2 additions & 2 deletions lib/coverband/reporters/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def fix_reports(reports)

# apply coverband filters
report_files.each_pair do |file, data|
next if Coverband.configuration.ignore.any? { |i| file.match(i) }
next if Coverband.configuration.ignore.any? { |i| file.match?(i) }

filtered_report_files[report_name][file] = data
end
Expand Down Expand Up @@ -93,7 +93,7 @@ def get_current_scov_data_imp(store, roots, options = {})
scov_style_report = {}
store.get_coverage_report(options).each_pair do |name, data|
data.each_pair do |key, line_data|
next if Coverband.configuration.ignore.any? { |i| key.match(i) }
next if Coverband.configuration.ignore.any? { |i| key.match?(i) }
next unless line_data

scov_style_report[name] = {} unless scov_style_report.key?(name)
Expand Down
7 changes: 4 additions & 3 deletions lib/coverband/utils/source_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ def no_lines?
end

def lines_strength
lines.map(&:coverage).compact.reduce(:+)
lines.sum do |line|
line.coverage || 0
end
end

def relevant_lines
Expand Down Expand Up @@ -256,8 +258,7 @@ def process_skipped_lines(lines)
# was at the start of the file name
# I had previously patched this in my local Rails app
def short_name
filename.sub(/^#{Coverband.configuration.root}/, ".")
.gsub(%r{^\.\/}, "")
filename.delete_prefix("#{Coverband.configuration.root}/")
end

def relative_path
Expand Down
6 changes: 3 additions & 3 deletions test/coverband/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def setup

test "ignore works with equal" do
Coverband::Collectors::Coverage.instance.reset_instance
expected = ["vendor/", ".erb$", ".slim$", "/tmp", "internal:prelude", "db/schema.rb", "config/environments"]
expected = ["vendor/", ".erb$", ".slim$", "/tmp", "internal:prelude", "db/schema.rb", "config/environments"].map { |str| Regexp.new(str) }
assert_equal expected, Coverband.configuration.ignore
end

Expand All @@ -34,7 +34,7 @@ def setup
"internal:prelude",
"db/schema.rb",
"config/environments",
"config/initializers"]
"config/initializers"].map { |str| Regexp.new(str) }
assert_equal expected, Coverband.configuration.ignore
end

Expand All @@ -44,7 +44,7 @@ def setup
config.ignore = ["*invalidRegex*"]
end
Coverband::Collectors::Coverage.instance.reset_instance
expected = Coverband::Configuration::IGNORE_DEFAULTS << "config/environments"
expected = (Coverband::Configuration::IGNORE_DEFAULTS << "config/environments").map { |str| Regexp.new(str) }
assert_equal expected, Coverband.configuration.ignore
end

Expand Down