Skip to content

Commit ab3d5e2

Browse files
committed
Lift dogfood to 100% method coverage
Enable :method in after(:suite) and threshold all three criteria. Two real fixes fell out: restore SourceFile#parse_ruby_array_string's 'try plain Ripper, then pre-quote on failure' approach (method-coverage keys for simplecov-on-simplecov take the form ["#<Class:SimpleCov>", :name, ...] — valid Ruby with the wrapper inside a quoted string), and merge singleton + instance method-coverage keys in ResultAdapter#normalize_method_keys (Ruby's Coverage records module_function methods twice; only one form is reachable, so the other was always 0).
1 parent dbd28cb commit ab3d5e2

4 files changed

Lines changed: 50 additions & 14 deletions

File tree

lib/simplecov/result_adapter.rb

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,29 @@ def adapt
4343
ADDRESS_PLACEHOLDER = "0x0"
4444
private_constant :ADDRESS_PLACEHOLDER
4545

46+
# Strip the `#<Class:Foo>` wrapper Ruby's Coverage adds to singleton-class
47+
# method keys. `module_function` and class methods get recorded both as
48+
# singleton (`[#<Class:Foo>, :m, …]`) and instance/module (`[Foo, :m, …]`)
49+
# entries pointing at the same source location; only one of the two is
50+
# ever reachable at runtime, so we merge them. Only applies to named
51+
# constants — anonymous-class addresses like `#<Class:0x0>` are left
52+
# alone (handled by ADDRESS_PATTERN above).
53+
SINGLETON_WRAPPER_PATTERN = /\A#<Class:([A-Z_][\w:]*)>\z/
54+
private_constant :SINGLETON_WRAPPER_PATTERN
55+
4656
def normalize_method_keys(cover_statistic)
4757
methods = cover_statistic[:methods]
4858
return unless methods
4959

50-
normalized = {}
51-
methods.each do |key, count|
60+
cover_statistic[:methods] = methods.each_with_object({}) do |(key, count), normalized|
5261
normalized_key = key.dup
53-
normalized_key[0] = normalized_key[0].to_s.gsub(ADDRESS_PATTERN, ADDRESS_PLACEHOLDER)
54-
# Keys might collide after normalization (two anonymous classes with same method)
62+
normalized_key[0] = key[0].to_s
63+
.gsub(ADDRESS_PATTERN, ADDRESS_PLACEHOLDER)
64+
.sub(SINGLETON_WRAPPER_PATTERN, '\1')
65+
# Keys may collide after normalization (anonymous classes sharing a
66+
# method name, or singleton + instance forms of a module_function method).
5567
normalized[normalized_key] = normalized.fetch(normalized_key, 0) + count
5668
end
57-
cover_statistic[:methods] = normalized
5869
end
5970

6071
def adapt_oneshot_lines_if_needed(file_name, cover_statistic)

lib/simplecov/source_file.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,12 @@ def restore_ruby_data_structure(structure)
363363
# Ripper to walk the literal so we don't need to hand-roll a scanner
364364
# for symbols, strings, integers, and constant paths.
365365
def parse_ruby_array_string(str)
366-
sexp = Ripper.sexp(quote_inspected_class_segments(str))
367-
# simplecov:disable — defensive: Ripper.sexp returning nil requires malformed input
366+
# Try plain Ripper first; only pre-quote `#<...>` inspect segments
367+
# if the input isn't already valid Ruby (otherwise we corrupt
368+
# `"#<Class:Foo>"` strings that *are* valid Ruby literals — exactly
369+
# the shape simplecov-on-simplecov method-coverage keys take).
370+
sexp = Ripper.sexp(str) || Ripper.sexp(quote_inspected_class_segments(str))
371+
# simplecov:disable — defensive: Ripper.sexp returning nil from both passes requires malformed input
368372
array_node = sexp&.dig(1, 0)
369373
# simplecov:enable
370374
raise ArgumentError, "expected array literal: #{str.inspect}" unless array_node && array_node[0] == :array

spec/helper.rb

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
# dependency's behaviour around Coverage is being investigated.
77
unless ENV["SIMPLECOV_NO_DOGFOOD"]
88
require "coverage"
9-
Coverage.start(lines: true, branches: true)
9+
Coverage.start(lines: true, branches: true, methods: true)
1010
end
1111

1212
require "rspec"
@@ -38,19 +38,20 @@
3838
raw = SimpleCov::UselessResultsRemover.call(Coverage.result)
3939
adapted = SimpleCov::ResultAdapter.call(raw)
4040

41-
# Enabling :branch is what teaches FileList / Result to surface
42-
# the branch data in coverage_statistics. We enable it here
43-
# (rather than in SimpleCov.start) to avoid leaking branch-mode
44-
# output shape into formatter specs that assert against
45-
# line-only fixtures.
41+
# Enabling :branch / :method is what teaches FileList / Result
42+
# to surface those data in coverage_statistics. We enable here
43+
# (rather than in SimpleCov.start) to avoid leaking the
44+
# multi-criterion output shape into formatter specs that assert
45+
# against line-only fixtures.
4646
SimpleCov.enable_coverage :branch if SimpleCov.branch_coverage_supported?
47+
SimpleCov.enable_coverage :method if SimpleCov.method_coverage_supported?
4748
result = SimpleCov::Result.new(adapted, filters: SimpleCov.filters + extra_filters, groups: {})
4849

4950
SimpleCov::Formatter::HTMLFormatter.new(silent: true, output_dir: DOGFOOD_OUTPUT_DIR).format(result)
5051

5152
shortfalls = []
5253
stats = result.coverage_statistics
53-
%i[line branch].each do |criterion|
54+
%i[line branch method].each do |criterion|
5455
actual = stats[criterion]&.percent
5556
next if actual.nil? || actual >= DOGFOOD_MINIMUM_COVERAGE
5657

spec/result_adapter_spec.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,26 @@
123123
end
124124
end
125125

126+
context "with module_function double-counting (singleton + instance forms)" do
127+
let(:result_set) do
128+
{
129+
existing_file => {
130+
methods: {
131+
["#<Class:SimpleCov::Combine>", :combine, 16, 4, 20, 7] => 5,
132+
[SimpleCov::Combine, :combine, 16, 4, 20, 7] => 0
133+
}
134+
}
135+
}
136+
end
137+
138+
it "merges singleton and instance entries into a single key with combined hits" do
139+
methods = adapter[existing_file][:methods]
140+
expect(methods.keys.size).to eq(1)
141+
expect(methods.keys.first[0]).to eq("SimpleCov::Combine")
142+
expect(methods.values.first).to eq(5)
143+
end
144+
end
145+
126146
context "with two distinct anonymous classes that share a method" do
127147
let(:result_set) do
128148
{

0 commit comments

Comments
 (0)