Skip to content

Commit

Permalink
Use CoverageData everywhere for consistency
Browse files Browse the repository at this point in the history
This allows us to get rid off duplicated and inconsistent
calculations. As a side effect of this, #565 is fixed now.

So, fixes #565.

The bigger purpose is multi step though where the access
to coverage data is normalized and hence methods that work
with expecting a minimum amount of coverage and formatters
can work with the known good same data base as opposed
to memoizing all different method names.

At first I wanted to only do it for FileList and leave the
rest for later. But the strength computation needed the
total number of lines of code and so to pass it in I'd have
had to calculate it but I wanted to calculate it only in
CoverageData and there the avalanche started....

Commit also includes moving methods around in SourceFile to be
private - it happened in the moment, forgive me for no extra
commit (it was hard to determine what I could delete/what I need
to adjust etc.).
  • Loading branch information
PragTob committed Jan 25, 2020
1 parent 6eef92c commit fab30ce
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 159 deletions.
27 changes: 25 additions & 2 deletions lib/simplecov/coverage_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,44 @@ module SimpleCov
class CoverageData
attr_reader :total, :covered, :missed, :strength, :percent

def self.from(coverage_data)
covered, missed, total_strength =
coverage_data.reduce([0, 0, 0.0]) do |(covered, missed, total_strength), file_coverage_data|
[
covered + file_coverage_data.covered,
missed + file_coverage_data.missed,
# gotta remultiply with loc because files have different strenght and loc
# giving them a different "weight" in total
total_strength + (file_coverage_data.strength * file_coverage_data.total)
]
end

new(covered: covered, missed: missed, total_strength: total_strength)
end

# Requires only covered, missed and strength to be initialized.
#
# Other values are computed by this class.
def initialize(covered:, missed:, strength: nil)
def initialize(covered:, missed:, total_strength: 0.0)
@covered = covered
@missed = missed
@strength = strength
@total = covered + missed
@percent = compute_percent(covered, total)
@strength = compute_strength(total_strength, @total)
end

private

def compute_percent(covered, total)
return 100.0 if total.zero?

Float(covered * 100.0 / total)
end

def compute_strength(total_strength, total)
return 0.0 if total.zero?

Float(total_strength.to_f / total)

This comment has been minimized.

Copy link
@krtschmr

krtschmr Jan 25, 2020

float divided will always return a float no need to cast?

This comment has been minimized.

Copy link
@PragTob

PragTob Jan 25, 2020

Author Collaborator

Yeah, I translated this from the previous code and was a bit baffled by this but didn't wanna take my chances in changing it. It's the same above as well (the * 100.0) should make it a float 🤷‍♂️

I guess we should be able to remove it, you're right. Thanks!

This comment has been minimized.

Copy link
@bf4

bf4 Jan 26, 2020

Collaborator

There was a segfault with mathn at one point

This comment has been minimized.

Copy link
@PragTob

PragTob Jan 26, 2020

Author Collaborator

weird, thanks!

end
end
end
69 changes: 19 additions & 50 deletions lib/simplecov/file_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class FileList
# also delegating methods implemented in Enumerable as they have
# custom Array implementations which are presumably better/more
# resource efficient
:size, :map,
:size, :map, :count,
# surprisingly not in Enumerable
:empty?, :length,
# still act like we're kinda an array
Expand All @@ -24,24 +24,17 @@ def initialize(files)
end

def coverage
{
**line_coverage,
**branch_coverage
}
@coverage ||= compute_coverage
end

# Returns the count of lines that have coverage
def covered_lines
return 0.0 if empty?

map { |f| f.covered_lines.count }.inject(:+)
coverage[:line]&.covered
end

# Returns the count of lines that have been missed
def missed_lines
return 0.0 if empty?

map { |f| f.missed_lines.count }.inject(:+)
coverage[:line]&.missed
end

# Returns the count of lines that are not relevant for coverage
Expand Down Expand Up @@ -71,75 +64,51 @@ def least_covered_file

# Returns the overall amount of relevant lines of code across all files in this list
def lines_of_code
covered_lines + missed_lines
coverage[:line]&.total
end

# Computes the coverage based upon lines covered and lines missed
# @return [Float]
def covered_percent
percent(covered_lines, lines_of_code)
coverage[:line]&.percent
end

# Computes the strength (hits / line) based upon lines covered and lines missed
# @return [Float]
def covered_strength
return 0.0 if empty? || lines_of_code.zero?

Float(map { |f| f.covered_strength * f.lines_of_code }.inject(:+) / lines_of_code)
coverage[:line]&.strength
end

# Return total count of branches in all files
def total_branches
return 0 if empty?

map { |file| file.total_branches.count }.inject(:+)
coverage[:branch]&.total
end

# Return total count of covered branches
def covered_branches
return 0 if empty?

map { |file| file.covered_branches.count }.inject(:+)
coverage[:branch]&.covered
end

# Return total count of covered branches
def missed_branches
return 0 if empty?

map { |file| file.missed_branches.count }.inject(:+)
coverage[:branch]&.missed
end

def branch_covered_percent
percent(covered_branches, total_branches)
coverage[:branch]&.percent
end

private

def percent(covered, total)
return 100.0 if empty? || total.zero?

Float(covered * 100.0 / total)
end

def line_coverage
{
line: CoverageData.new(
strength: covered_strength,
covered: covered_lines,
missed: missed_lines
)
}
end

def branch_coverage
return nil unless SimpleCov.branch_coverage?
def compute_coverage
total_coverage_data = @files.each_with_object(line: [], branch: []) do |file, together|
together[:line] << file.coverage[:line]
together[:branch] << file.coverage[:branch] if SimpleCov.branch_coverage?
end

{
branch: CoverageData.new(
covered: covered_branches,
missed: missed_branches
)
}
coverage_data = {line: CoverageData.from(total_coverage_data[:line])}
coverage_data[:branch] = CoverageData.from(total_coverage_data[:branch]) if SimpleCov.branch_coverage?
coverage_data
end
end
end
Loading

0 comments on commit fab30ce

Please sign in to comment.