From 6026142f9fb00690e8732ecac9f3c654d2a21941 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Wed, 9 Aug 2017 17:24:19 +0100 Subject: [PATCH 1/3] Classify lines from unloaded file as relevant or not relevant Lines with whitespace, comments, or blocks with :nocov: should not be relevant (nil). Other lines from unloaded files should be relevant with zero cover (0). Fixes #604 --- features/config_tracked_files.feature | 2 +- ...onfig_tracked_files_relevant_lines.feature | 31 +++++++++ lib/simplecov.rb | 5 +- lib/simplecov/lines_classifier.rb | 29 +++++++++ spec/lines_classifier_spec.rb | 64 +++++++++++++++++++ 5 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 features/config_tracked_files_relevant_lines.feature create mode 100644 lib/simplecov/lines_classifier.rb create mode 100644 spec/lines_classifier_spec.rb diff --git a/features/config_tracked_files.feature b/features/config_tracked_files.feature index ba0c4f0c..be701420 100644 --- a/features/config_tracked_files.feature +++ b/features/config_tracked_files.feature @@ -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 | diff --git a/features/config_tracked_files_relevant_lines.feature b/features/config_tracked_files_relevant_lines.feature new file mode 100644 index 00000000..ed13a73b --- /dev/null +++ b/features/config_tracked_files_relevant_lines.feature @@ -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" diff --git a/lib/simplecov.rb b/lib/simplecov.rb index d4247068..3ba46987 100644 --- a/lib/simplecov.rb +++ b/lib/simplecov.rb @@ -55,7 +55,7 @@ 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 @@ -63,7 +63,7 @@ def add_not_loaded_files(result) 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 @@ -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" diff --git a/lib/simplecov/lines_classifier.rb b/lib/simplecov/lines_classifier.rb new file mode 100644 index 00000000..87274c38 --- /dev/null +++ b/lib/simplecov/lines_classifier.rb @@ -0,0 +1,29 @@ +module SimpleCov + 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 + elsif line =~ WHITESPACE_OR_COMMENT_LINE || skipping + NOT_RELEVANT + else + RELEVANT + end + end + end + end +end diff --git a/spec/lines_classifier_spec.rb b/spec/lines_classifier_spec.rb new file mode 100644 index 00000000..0a6a3455 --- /dev/null +++ b/spec/lines_classifier_spec.rb @@ -0,0 +1,64 @@ +require "helper" +require "simplecov/lines_classifier" + +describe SimpleCov::LinesClassifier do + describe "#classify" do + describe "only relevant lines" do + it "classifies each line as relevant" do + classified_lines = subject.classify [ + "def foo", + "end", + ] + + expect(classified_lines.length).to eq 2 + expect(classified_lines).to all be_relevant + end + end + + describe "not-relevant lines" do + it "classifies whitespace as not-relevant" do + classified_lines = subject.classify [ + "", + " ", + ] + + expect(classified_lines.length).to eq 2 + expect(classified_lines).to all be_not_relevant + end + + it "classifies comments as not-relevant" do + classified_lines = subject.classify [ + "#Comment", + " # Leading space comment", + ] + + expect(classified_lines.length).to eq 2 + expect(classified_lines).to all be_not_relevant + end + + it "classifies :nocov: blocks as 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_not_relevant + end + end + end + + RSpec::Matchers.define :be_relevant do + match do |actual| + actual == SimpleCov::LinesClassifier::RELEVANT + end + end + + RSpec::Matchers.define :be_not_relevant do + match do |actual| + actual == SimpleCov::LinesClassifier::NOT_RELEVANT + end + end +end From 86a8c2a65de4ea255be61a6331336f9ffdaaba80 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Wed, 9 Aug 2017 17:40:58 +0100 Subject: [PATCH 2/3] Unify definition of no_cov token regex --- lib/simplecov/source_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/simplecov/source_file.rb b/lib/simplecov/source_file.rb index 33eb9fb1..5e358503 100644 --- a/lib/simplecov/source_file.rb +++ b/lib/simplecov/source_file.rb @@ -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 From 8cd5578e923afc5c192d88eee7483fa235cf6dc5 Mon Sep 17 00:00:00 2001 From: odlp Date: Sat, 12 Aug 2017 12:53:26 +0100 Subject: [PATCH 3/3] Expand lines classifier unit tests Adds examples for hard tabs, string interpolation and non-closing :nocov: tokens. Also adds a descriptive comment to SimpleCov::LinesClassifier. --- lib/simplecov/lines_classifier.rb | 5 +- spec/lines_classifier_spec.rb | 89 ++++++++++++++++++++++--------- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/lib/simplecov/lines_classifier.rb b/lib/simplecov/lines_classifier.rb index 87274c38..cd80c8a1 100644 --- a/lib/simplecov/lines_classifier.rb +++ b/lib/simplecov/lines_classifier.rb @@ -1,4 +1,7 @@ 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 @@ -18,7 +21,7 @@ def classify(lines) if line =~ self.class.no_cov_line skipping = !skipping NOT_RELEVANT - elsif line =~ WHITESPACE_OR_COMMENT_LINE || skipping + elsif skipping || line =~ WHITESPACE_OR_COMMENT_LINE NOT_RELEVANT else RELEVANT diff --git a/spec/lines_classifier_spec.rb b/spec/lines_classifier_spec.rb index 0a6a3455..a6e7b7e3 100644 --- a/spec/lines_classifier_spec.rb +++ b/spec/lines_classifier_spec.rb @@ -3,49 +3,88 @@ describe SimpleCov::LinesClassifier do describe "#classify" do - describe "only relevant lines" do - it "classifies each line as relevant" do + describe "relevant lines" do + it "determines code as relevant" do classified_lines = subject.classify [ - "def foo", + "module Foo", + " class Baz", + " def Bar", + " puts 'hi'", + " end", + " end", "end", ] - expect(classified_lines.length).to eq 2 + expect(classified_lines.length).to eq 7 expect(classified_lines).to all be_relevant end end describe "not-relevant lines" do - it "classifies whitespace as not-relevant" do + it "determines whitespace is not-relevant" do classified_lines = subject.classify [ "", - " ", + " ", + "\t\t", ] - expect(classified_lines.length).to eq 2 - expect(classified_lines).to all be_not_relevant + expect(classified_lines.length).to eq 3 + expect(classified_lines).to all be_irrelevant end - it "classifies comments as not-relevant" do - classified_lines = subject.classify [ - "#Comment", - " # Leading space comment", - ] + 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 - expect(classified_lines.length).to eq 2 - expect(classified_lines).to all be_not_relevant + 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 - it "classifies :nocov: blocks as not-relevant" do - classified_lines = subject.classify [ - "# :nocov:", - "def hi", - "end", - "# :nocov:", - ] + 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.length).to eq 4 - expect(classified_lines).to all be_not_relevant + 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 @@ -56,7 +95,7 @@ end end - RSpec::Matchers.define :be_not_relevant do + RSpec::Matchers.define :be_irrelevant do match do |actual| actual == SimpleCov::LinesClassifier::NOT_RELEVANT end