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

Fix relevant lines for unloaded files #605

Merged
merged 3 commits into from
Aug 12, 2017
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
2 changes: 1 addition & 1 deletion features/config_tracked_files.feature
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Feature:
When I open the coverage report generated with `bundle exec rake test`
Then I should see the groups:
| name | coverage | files |
| All Files | 76.81% | 7 |
| All Files | 77.94% | 7 |

And I should see the source files:
| name | coverage |
Expand Down
31 changes: 31 additions & 0 deletions features/config_tracked_files_relevant_lines.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
@rspec
Feature:

Using the setting `tracked_files` should classify whether lines
are relevant or not (such as whitespace or comments).

Scenario:
Given SimpleCov for RSpec is configured with:
"""
require 'simplecov'
SimpleCov.start do
track_files "lib/**/*.rb"
end
"""
Given a file named "lib/not_loaded.rb" with:
"""
# A comment line. Plus a whitespace line below:

# :nocov:
def ignore_me
end
# :nocov:

def this_is_relevant
puts "still relevant"
end
"""

When I open the coverage report generated with `bundle exec rspec spec`
Then I follow "lib/not_loaded.rb"
Then I should see "3 relevant lines" within ".highlighted"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for adding an integration test! 👍

5 changes: 3 additions & 2 deletions lib/simplecov.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ def start(profile = nil, &block)

#
# Finds files that were to be tracked but were not loaded and initializes
# their coverage to zero.
# the line-by-line coverage to zero (if relevant) or nil (comments / whitespace etc).
#
def add_not_loaded_files(result)
if tracked_files
result = result.dup
Dir[tracked_files].each do |file|
absolute = File.expand_path(file)

result[absolute] ||= [0] * File.foreach(absolute).count
result[absolute] ||= LinesClassifier.new.classify(File.foreach(absolute))
end
end

Expand Down Expand Up @@ -177,6 +177,7 @@ def clear_result
require "simplecov/filter"
require "simplecov/formatter"
require "simplecov/last_run"
require "simplecov/lines_classifier"
require "simplecov/raw_coverage"
require "simplecov/result_merger"
require "simplecov/command_guesser"
Expand Down
32 changes: 32 additions & 0 deletions lib/simplecov/lines_classifier.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
module SimpleCov
# Classifies whether lines are relevant for code coverage analysis.
# Comments & whitespace lines, and :nocov: token blocks, are considered not relevant.

class LinesClassifier
RELEVANT = 0
NOT_RELEVANT = nil

WHITESPACE_LINE = /^\s*$/
COMMENT_LINE = /^\s*#/
WHITESPACE_OR_COMMENT_LINE = Regexp.union(WHITESPACE_LINE, COMMENT_LINE)

def self.no_cov_line
/^(\s*)#(\s*)(\:#{SimpleCov.nocov_token}\:)/
end

def classify(lines)
skipping = false

lines.map do |line|
if line =~ self.class.no_cov_line
skipping = !skipping
NOT_RELEVANT
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 using constants!

elsif skipping || line =~ WHITESPACE_OR_COMMENT_LINE
NOT_RELEVANT
else
RELEVANT
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/simplecov/source_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def process_skipped_lines(lines)
skipping = false

lines.each do |line|
if line.src =~ /^([\s]*)#([\s]*)(\:#{SimpleCov.nocov_token}\:)/
if line.src =~ SimpleCov::LinesClassifier.no_cov_line
skipping = !skipping
line.skipped!
elsif skipping
Expand Down
103 changes: 103 additions & 0 deletions spec/lines_classifier_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
require "helper"
require "simplecov/lines_classifier"

describe SimpleCov::LinesClassifier do
describe "#classify" do
describe "relevant lines" do
it "determines code as relevant" do
classified_lines = subject.classify [
"module Foo",
" class Baz",
" def Bar",
" puts 'hi'",
" end",
" end",
"end",
]

expect(classified_lines.length).to eq 7
expect(classified_lines).to all be_relevant
end
end

describe "not-relevant lines" do
it "determines whitespace is not-relevant" do
classified_lines = subject.classify [
"",
" ",
"\t\t",
]

expect(classified_lines.length).to eq 3
expect(classified_lines).to all be_irrelevant
end

describe "comments" do
it "determines comments are not-relevant" do
classified_lines = subject.classify [
"#Comment",
" # Leading space comment",
"\t# Leading tab comment",
]

expect(classified_lines.length).to eq 3
expect(classified_lines).to all be_irrelevant
end

it "doesn't mistake interpolation as a comment" do
classified_lines = subject.classify [
'puts "#{var}"',
]

expect(classified_lines.length).to eq 1
expect(classified_lines).to all be_relevant
end
end

describe ":nocov: blocks" do
it "determines :nocov: blocks are not-relevant" do
classified_lines = subject.classify [
"# :nocov:",
"def hi",
"end",
"# :nocov:",
]

expect(classified_lines.length).to eq 4
expect(classified_lines).to all be_irrelevant
end

it "determines all lines after a non-closing :nocov: as not-relevant" do
classified_lines = subject.classify [
"# :nocov:",
"puts 'Not relevant'",
"# :nocov:",
"puts 'Relevant again'",
"puts 'Still relevant'",
"# :nocov:",
"puts 'Not relevant till the end'",
"puts 'Ditto'",
]

expect(classified_lines.length).to eq 8

expect(classified_lines[0..2]).to all be_irrelevant
expect(classified_lines[3..4]).to all be_relevant
expect(classified_lines[5..7]).to all be_irrelevant
end
end
end
end

RSpec::Matchers.define :be_relevant do
match do |actual|
actual == SimpleCov::LinesClassifier::RELEVANT
end
end

RSpec::Matchers.define :be_irrelevant do
match do |actual|
actual == SimpleCov::LinesClassifier::NOT_RELEVANT
end
end
end