From 9bb1aa3a6735b16d4026c4c34db0de6973d9a883 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 14 Feb 2017 10:01:03 -0500 Subject: [PATCH 01/10] Rewrite merge helpers to work with frozen objects Coverage.result returns a frozen Hash containing frozen Strings as keys and frozen Arrays as values. The existing merge helper did not work when passed a Coverage.result directly, because it would try to extend the frozen Arrays. This is not a problem in typical SimpleCov usage because usually Coverage.result has been roundtripped through .resultset.json resulting in an unfrozen copy. But when trying to do manual merging of Coverage.result hashes the use of extend gets in the way. The merge helper has now been rewritten a method that takes two Hashes and doesn't modify them or their contents at all. While I was touching this code I tried to clean it up a little bit as well. --- lib/simplecov.rb | 2 +- lib/simplecov/merge_helper.rb | 31 ++++++++++++++++ lib/simplecov/merge_helpers.rb | 37 ------------------- lib/simplecov/result.rb | 1 - lib/simplecov/result_merger.rb | 2 +- ...e_helpers_spec.rb => merge_helper_spec.rb} | 24 ++++++++++-- 6 files changed, 54 insertions(+), 43 deletions(-) create mode 100644 lib/simplecov/merge_helper.rb delete mode 100644 lib/simplecov/merge_helpers.rb rename spec/{merge_helpers_spec.rb => merge_helper_spec.rb} (86%) diff --git a/lib/simplecov.rb b/lib/simplecov.rb index cb9f685f..a02d4cbd 100644 --- a/lib/simplecov.rb +++ b/lib/simplecov.rb @@ -172,7 +172,7 @@ def usable? require "simplecov/filter" require "simplecov/formatter" require "simplecov/last_run" -require "simplecov/merge_helpers" +require "simplecov/merge_helper" require "simplecov/result_merger" require "simplecov/command_guesser" require "simplecov/version" diff --git a/lib/simplecov/merge_helper.rb b/lib/simplecov/merge_helper.rb new file mode 100644 index 00000000..6dbe56ef --- /dev/null +++ b/lib/simplecov/merge_helper.rb @@ -0,0 +1,31 @@ +module SimpleCov + module MergeHelper + class << self + # Merges two Coverage.result hashes + def merge_resultsets(a, b) + (a.keys | b.keys).each_with_object({}) do |filename, merged| + result1 = a[filename] + result2 = b[filename] + merged[filename] = if result1 && result2 + merge_arrays(result1, result2) + else + (result1 || result2).dup + end + end + end + + private + + def merge_arrays(a, b) + a.zip(b).map do |count1, count2| + sum = count1.to_i + count2.to_i + if sum.zero? && (count1.nil? || count2.nil?) + nil + else + sum + end + end + end + end + end +end diff --git a/lib/simplecov/merge_helpers.rb b/lib/simplecov/merge_helpers.rb deleted file mode 100644 index 67802e4e..00000000 --- a/lib/simplecov/merge_helpers.rb +++ /dev/null @@ -1,37 +0,0 @@ -module SimpleCov - module ArrayMergeHelper - # Merges an array of coverage results with self - def merge_resultset(array) - new_array = dup - array.each_with_index do |element, i| - pair = [element, new_array[i]] - new_array[i] = if pair.any?(&:nil?) && pair.map(&:to_i).all?(&:zero?) - nil - else - element.to_i + new_array[i].to_i - end - end - new_array - end - end -end - -module SimpleCov - module HashMergeHelper - # Merges the given Coverage.result hash with self - def merge_resultset(hash) - new_resultset = {} - (keys + hash.keys).each do |filename| - new_resultset[filename] = nil - end - - new_resultset.each_key do |filename| - result1 = self[filename] - result2 = hash[filename] - new_resultset[filename] = - result1 && result2 ? result1.extend(ArrayMergeHelper).merge_resultset(result2) : (result1 || result2).dup - end - new_resultset - end - end -end diff --git a/lib/simplecov/result.rb b/lib/simplecov/result.rb index 29eecd5c..badf182f 100644 --- a/lib/simplecov/result.rb +++ b/lib/simplecov/result.rb @@ -24,7 +24,6 @@ class Result # Initialize a new SimpleCov::Result from given Coverage.result (a Hash of filenames each containing an array of # coverage data) def initialize(original_result) - original_result = original_result.dup.extend(SimpleCov::HashMergeHelper) unless original_result.is_a? SimpleCov::HashMergeHelper @original_result = original_result.freeze @files = SimpleCov::FileList.new(original_result.map do |filename, coverage| SimpleCov::SourceFile.new(filename, coverage) if File.file?(filename) diff --git a/lib/simplecov/result_merger.rb b/lib/simplecov/result_merger.rb index 6660e5cf..b63cc340 100644 --- a/lib/simplecov/result_merger.rb +++ b/lib/simplecov/result_merger.rb @@ -62,7 +62,7 @@ def results def merged_result merged = {} results.each do |result| - merged = result.original_result.merge_resultset(merged) + merged = SimpleCov::MergeHelper.merge_resultsets(result.original_result, merged) end result = SimpleCov::Result.new(merged) # Specify the command name diff --git a/spec/merge_helpers_spec.rb b/spec/merge_helper_spec.rb similarity index 86% rename from spec/merge_helpers_spec.rb rename to spec/merge_helper_spec.rb index d4eaa50e..6e4b172a 100644 --- a/spec/merge_helpers_spec.rb +++ b/spec/merge_helper_spec.rb @@ -1,7 +1,7 @@ require "helper" if SimpleCov.usable? - describe "merge helpers" do + describe "merge helper" do describe "with two faked coverage resultsets" do before do SimpleCov.use_merging true @@ -12,7 +12,7 @@ source_fixture("resultset1.rb") => [1, 1, 1, 1], source_fixture("parallel_tests.rb") => [nil, 0, nil, 0], source_fixture("conditionally_loaded_1.rb") => [nil, 0, 1], # loaded only in the first resultset - }.extend(SimpleCov::HashMergeHelper) + } @resultset2 = { source_fixture("sample.rb") => [1, nil, 1, 1, nil, nil, 1, 1, nil, nil], @@ -26,7 +26,7 @@ context "a merge" do subject do - @resultset1.merge_resultset(@resultset2) + SimpleCov::MergeHelper.merge_resultsets(@resultset1, @resultset2) end it "has proper results for sample.rb" do @@ -122,5 +122,23 @@ end end end + + describe "with frozen resultsets" do + before do + @resultset1 = { + source_fixture("sample.rb").freeze => [nil, 1, 1, 1, nil, nil, 1, 1, nil, nil].freeze, + source_fixture("app/models/user.rb").freeze => [nil, 1, 1, 1, nil, nil, 1, 0, nil, nil].freeze, + }.freeze + + @resultset2 = { + source_fixture("sample.rb").freeze => [1, nil, 1, 1, nil, nil, 1, 1, nil, nil].freeze, + source_fixture("app/models/user.rb").freeze => [nil, 1, 5, 1, nil, nil, 1, 0, nil, nil].freeze, + }.freeze + end + + it "can merge without exceptions" do + SimpleCov::MergeHelper.merge_resultsets(@resultset1, @resultset2) + end + end end end From a7747c1391b7f8d0b7081c23df79b151ebbc95bd Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 14 Feb 2017 13:58:27 -0500 Subject: [PATCH 02/10] MergeHelper.merge_resultsets -> RawCoverage.merge_results --- lib/simplecov.rb | 2 +- lib/simplecov/{merge_helper.rb => raw_coverage.rb} | 4 ++-- lib/simplecov/result_merger.rb | 2 +- spec/{merge_helper_spec.rb => raw_coverage_spec.rb} | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) rename lib/simplecov/{merge_helper.rb => raw_coverage.rb} (93%) rename spec/{merge_helper_spec.rb => raw_coverage_spec.rb} (96%) diff --git a/lib/simplecov.rb b/lib/simplecov.rb index a02d4cbd..3e3ed5e8 100644 --- a/lib/simplecov.rb +++ b/lib/simplecov.rb @@ -172,7 +172,7 @@ def usable? require "simplecov/filter" require "simplecov/formatter" require "simplecov/last_run" -require "simplecov/merge_helper" +require "simplecov/raw_coverage" require "simplecov/result_merger" require "simplecov/command_guesser" require "simplecov/version" diff --git a/lib/simplecov/merge_helper.rb b/lib/simplecov/raw_coverage.rb similarity index 93% rename from lib/simplecov/merge_helper.rb rename to lib/simplecov/raw_coverage.rb index 6dbe56ef..f832a3d2 100644 --- a/lib/simplecov/merge_helper.rb +++ b/lib/simplecov/raw_coverage.rb @@ -1,8 +1,8 @@ module SimpleCov - module MergeHelper + module RawCoverage class << self # Merges two Coverage.result hashes - def merge_resultsets(a, b) + def merge_results(a, b) (a.keys | b.keys).each_with_object({}) do |filename, merged| result1 = a[filename] result2 = b[filename] diff --git a/lib/simplecov/result_merger.rb b/lib/simplecov/result_merger.rb index b63cc340..88c1ea3e 100644 --- a/lib/simplecov/result_merger.rb +++ b/lib/simplecov/result_merger.rb @@ -62,7 +62,7 @@ def results def merged_result merged = {} results.each do |result| - merged = SimpleCov::MergeHelper.merge_resultsets(result.original_result, merged) + merged = SimpleCov::RawCoverage.merge_results(result.original_result, merged) end result = SimpleCov::Result.new(merged) # Specify the command name diff --git a/spec/merge_helper_spec.rb b/spec/raw_coverage_spec.rb similarity index 96% rename from spec/merge_helper_spec.rb rename to spec/raw_coverage_spec.rb index 6e4b172a..9b0bcf12 100644 --- a/spec/merge_helper_spec.rb +++ b/spec/raw_coverage_spec.rb @@ -1,7 +1,7 @@ require "helper" if SimpleCov.usable? - describe "merge helper" do + describe SimpleCov::RawCoverage do describe "with two faked coverage resultsets" do before do SimpleCov.use_merging true @@ -26,7 +26,7 @@ context "a merge" do subject do - SimpleCov::MergeHelper.merge_resultsets(@resultset1, @resultset2) + SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2) end it "has proper results for sample.rb" do @@ -137,7 +137,7 @@ end it "can merge without exceptions" do - SimpleCov::MergeHelper.merge_resultsets(@resultset1, @resultset2) + SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2) end end end From d08e5694f6a860061dd56858b54b07c6f55c9a12 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 14 Feb 2017 13:59:15 -0500 Subject: [PATCH 03/10] Split ResultMerger and RawCoverage specs into separate files --- spec/raw_coverage_spec.rb | 61 -------------------------- spec/result_merger_spec.rb | 88 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 61 deletions(-) create mode 100644 spec/result_merger_spec.rb diff --git a/spec/raw_coverage_spec.rb b/spec/raw_coverage_spec.rb index 9b0bcf12..1c08b81e 100644 --- a/spec/raw_coverage_spec.rb +++ b/spec/raw_coverage_spec.rb @@ -4,7 +4,6 @@ describe SimpleCov::RawCoverage do describe "with two faked coverage resultsets" do before do - SimpleCov.use_merging true @resultset1 = { source_fixture("sample.rb") => [nil, 1, 1, 1, nil, nil, 1, 1, nil, nil], source_fixture("app/models/user.rb") => [nil, 1, 1, 1, nil, nil, 1, 0, nil, nil], @@ -61,66 +60,6 @@ expect(subject[source_fixture("conditionally_loaded_2.rb")]).to eq([nil, 0, 1]) end end - - # See Github issue #6 - it "returns an empty hash when the resultset cache file is empty" do - File.open(SimpleCov::ResultMerger.resultset_path, "w+") { |f| f.puts "" } - expect(SimpleCov::ResultMerger.resultset).to be_empty - end - - # See Github issue #6 - it "returns an empty hash when the resultset cache file is not present" do - system "rm #{SimpleCov::ResultMerger.resultset_path}" if File.exist?(SimpleCov::ResultMerger.resultset_path) - expect(SimpleCov::ResultMerger.resultset).to be_empty - end - - context "and results generated from those" do - before do - system "rm #{SimpleCov::ResultMerger.resultset_path}" if File.exist?(SimpleCov::ResultMerger.resultset_path) - @result1 = SimpleCov::Result.new(@resultset1) - @result1.command_name = "result1" - @result2 = SimpleCov::Result.new(@resultset2) - @result2.command_name = "result2" - end - - context "with stored results" do - before do - SimpleCov::ResultMerger.store_result(@result1) - SimpleCov::ResultMerger.store_result(@result2) - end - - it "has stored data in resultset_path JSON file" do - expect(File.readlines(SimpleCov::ResultMerger.resultset_path).length).to be > 50 - end - - it "returns a hash containing keys ['result1' and 'result2'] for resultset" do - expect(SimpleCov::ResultMerger.resultset.keys.sort).to eq %w(result1 result2) - end - - it "returns proper values for merged_result" do - expect(SimpleCov::ResultMerger.merged_result.source_files.find { |s| s.filename =~ /user/ }.lines.map(&:coverage)).to eq([nil, 2, 6, 2, nil, nil, 2, 0, nil, nil]) - end - - context "with second result way above the merge_timeout" do - before do - @result2.created_at = Time.now - 172_800 # two days ago - SimpleCov::ResultMerger.store_result(@result2) - end - - it "has only one result in SimpleCov::ResultMerger.results" do - expect(SimpleCov::ResultMerger.results.length).to eq(1) - end - end - - context "with merging disabled" do - before { SimpleCov.use_merging false } - - it "returns nil for SimpleCov.result" do - expect(SimpleCov.result).to be_nil - end - end - end - end end describe "with frozen resultsets" do diff --git a/spec/result_merger_spec.rb b/spec/result_merger_spec.rb new file mode 100644 index 00000000..8bc6c9d6 --- /dev/null +++ b/spec/result_merger_spec.rb @@ -0,0 +1,88 @@ +require "helper" + +if SimpleCov.usable? + describe SimpleCov::ResultMerger do + describe "with two faked coverage resultsets" do + before do + SimpleCov.use_merging true + @resultset1 = { + source_fixture("sample.rb") => [nil, 1, 1, 1, nil, nil, 1, 1, nil, nil], + source_fixture("app/models/user.rb") => [nil, 1, 1, 1, nil, nil, 1, 0, nil, nil], + source_fixture("app/controllers/sample_controller.rb") => [nil, 1, 1, 1, nil, nil, 1, 0, nil, nil], + source_fixture("resultset1.rb") => [1, 1, 1, 1], + source_fixture("parallel_tests.rb") => [nil, 0, nil, 0], + source_fixture("conditionally_loaded_1.rb") => [nil, 0, 1], # loaded only in the first resultset + } + + @resultset2 = { + source_fixture("sample.rb") => [1, nil, 1, 1, nil, nil, 1, 1, nil, nil], + source_fixture("app/models/user.rb") => [nil, 1, 5, 1, nil, nil, 1, 0, nil, nil], + source_fixture("app/controllers/sample_controller.rb") => [nil, 3, 1, nil, nil, nil, 1, 0, nil, nil], + source_fixture("resultset2.rb") => [nil, 1, 1, nil], + source_fixture("parallel_tests.rb") => [nil, nil, 0, 0], + source_fixture("conditionally_loaded_2.rb") => [nil, 0, 1], # loaded only in the second resultset + } + end + + # See Github issue #6 + it "returns an empty hash when the resultset cache file is empty" do + File.open(SimpleCov::ResultMerger.resultset_path, "w+") { |f| f.puts "" } + expect(SimpleCov::ResultMerger.resultset).to be_empty + end + + # See Github issue #6 + it "returns an empty hash when the resultset cache file is not present" do + system "rm #{SimpleCov::ResultMerger.resultset_path}" if File.exist?(SimpleCov::ResultMerger.resultset_path) + expect(SimpleCov::ResultMerger.resultset).to be_empty + end + + context "and results generated from those" do + before do + system "rm #{SimpleCov::ResultMerger.resultset_path}" if File.exist?(SimpleCov::ResultMerger.resultset_path) + @result1 = SimpleCov::Result.new(@resultset1) + @result1.command_name = "result1" + @result2 = SimpleCov::Result.new(@resultset2) + @result2.command_name = "result2" + end + + context "with stored results" do + before do + SimpleCov::ResultMerger.store_result(@result1) + SimpleCov::ResultMerger.store_result(@result2) + end + + it "has stored data in resultset_path JSON file" do + expect(File.readlines(SimpleCov::ResultMerger.resultset_path).length).to be > 50 + end + + it "returns a hash containing keys ['result1' and 'result2'] for resultset" do + expect(SimpleCov::ResultMerger.resultset.keys.sort).to eq %w(result1 result2) + end + + it "returns proper values for merged_result" do + expect(SimpleCov::ResultMerger.merged_result.source_files.find { |s| s.filename =~ /user/ }.lines.map(&:coverage)).to eq([nil, 2, 6, 2, nil, nil, 2, 0, nil, nil]) + end + + context "with second result way above the merge_timeout" do + before do + @result2.created_at = Time.now - 172_800 # two days ago + SimpleCov::ResultMerger.store_result(@result2) + end + + it "has only one result in SimpleCov::ResultMerger.results" do + expect(SimpleCov::ResultMerger.results.length).to eq(1) + end + end + + context "with merging disabled" do + before { SimpleCov.use_merging false } + + it "returns nil for SimpleCov.result" do + expect(SimpleCov.result).to be_nil + end + end + end + end + end + end +end From c29bbcdae94632e3d2bc5bbeb471a388020fc3ad Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 14 Feb 2017 14:10:37 -0500 Subject: [PATCH 04/10] Add support for merging more than two Coverage.result hashes RawCoverage.merge_results can now merge an arbitrary number of hashes. --- lib/simplecov/raw_coverage.rb | 47 +++++++++++++++++++--------------- lib/simplecov/result_merger.rb | 5 +--- spec/raw_coverage_spec.rb | 12 ++++++++- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/lib/simplecov/raw_coverage.rb b/lib/simplecov/raw_coverage.rb index f832a3d2..6f6cba19 100644 --- a/lib/simplecov/raw_coverage.rb +++ b/lib/simplecov/raw_coverage.rb @@ -1,29 +1,34 @@ module SimpleCov module RawCoverage - class << self - # Merges two Coverage.result hashes - def merge_results(a, b) - (a.keys | b.keys).each_with_object({}) do |filename, merged| - result1 = a[filename] - result2 = b[filename] - merged[filename] = if result1 && result2 - merge_arrays(result1, result2) - else - (result1 || result2).dup - end - end + module_function + + # Merges multiple Coverage.result hashes + def merge_results(*results) + results.reduce({}) do |result, merged| + merge_resultsets(result, merged) end + end - private + # Merges two Coverage.result hashes + def merge_resultsets(a, b) + (a.keys | b.keys).each_with_object({}) do |filename, merged| + result1 = a[filename] + result2 = b[filename] + merged[filename] = merge_file_coverage(result1, result2) + end + end + + def merge_file_coverage(a, b) + unless a && b + return (a || b).dup + end - def merge_arrays(a, b) - a.zip(b).map do |count1, count2| - sum = count1.to_i + count2.to_i - if sum.zero? && (count1.nil? || count2.nil?) - nil - else - sum - end + a.zip(b).map do |count1, count2| + sum = count1.to_i + count2.to_i + if sum.zero? && (count1.nil? || count2.nil?) + nil + else + sum end end end diff --git a/lib/simplecov/result_merger.rb b/lib/simplecov/result_merger.rb index 88c1ea3e..314122f6 100644 --- a/lib/simplecov/result_merger.rb +++ b/lib/simplecov/result_merger.rb @@ -60,10 +60,7 @@ def results # for the result consisting of a join on all source result's names # def merged_result - merged = {} - results.each do |result| - merged = SimpleCov::RawCoverage.merge_results(result.original_result, merged) - end + merged = SimpleCov::RawCoverage.merge_results(*results.map(&:original_result)) result = SimpleCov::Result.new(merged) # Specify the command name result.command_name = results.map(&:command_name).sort.join(", ") diff --git a/spec/raw_coverage_spec.rb b/spec/raw_coverage_spec.rb index 1c08b81e..3674db17 100644 --- a/spec/raw_coverage_spec.rb +++ b/spec/raw_coverage_spec.rb @@ -11,6 +11,7 @@ source_fixture("resultset1.rb") => [1, 1, 1, 1], source_fixture("parallel_tests.rb") => [nil, 0, nil, 0], source_fixture("conditionally_loaded_1.rb") => [nil, 0, 1], # loaded only in the first resultset + source_fixture("three.rb") => [nil, 1, 1], } @resultset2 = { @@ -20,12 +21,17 @@ source_fixture("resultset2.rb") => [nil, 1, 1, nil], source_fixture("parallel_tests.rb") => [nil, nil, 0, 0], source_fixture("conditionally_loaded_2.rb") => [nil, 0, 1], # loaded only in the second resultset + source_fixture("three.rb") => [nil, 1, 4], + } + + @resultset3 = { + source_fixture("three.rb") => [nil, 1, 2], } end context "a merge" do subject do - SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2) + SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2, @resultset3) end it "has proper results for sample.rb" do @@ -59,6 +65,10 @@ it "has proper results for conditionally_loaded_2.rb" do expect(subject[source_fixture("conditionally_loaded_2.rb")]).to eq([nil, 0, 1]) end + + it "has proper results for three.rb" do + expect(subject[source_fixture("three.rb")]).to eq([nil, 3, 7]) + end end end From 7142edfebffd6277834d19c586886f1eb1a48cff Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 14 Feb 2017 14:16:41 -0500 Subject: [PATCH 05/10] Add ResultMerger.merge_results This just extracts the logic from .merged_result into a method that takes a set of SimpleCov::Results as input. This lets clients merge their own SimpleCov::Result objects without roundtripping through .resultset.json. --- lib/simplecov/result_merger.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/simplecov/result_merger.rb b/lib/simplecov/result_merger.rb index 314122f6..58f0dd47 100644 --- a/lib/simplecov/result_merger.rb +++ b/lib/simplecov/result_merger.rb @@ -54,17 +54,24 @@ def results results end + # Merge two or more SimpleCov::Results into a new one with merged + # coverage data and the command_name for the result consisting of a join + # on all source result's names + def merge_results(*results) + merged = SimpleCov::RawCoverage.merge_results(*results.map(&:original_result)) + result = SimpleCov::Result.new(merged) + # Specify the command name + result.command_name = results.map(&:command_name).sort.join(", ") + result + end + # # Gets all SimpleCov::Results from cache, merges them and produces a new # SimpleCov::Result with merged coverage data and the command_name # for the result consisting of a join on all source result's names # def merged_result - merged = SimpleCov::RawCoverage.merge_results(*results.map(&:original_result)) - result = SimpleCov::Result.new(merged) - # Specify the command name - result.command_name = results.map(&:command_name).sort.join(", ") - result + merge_results(*results) end # Saves the given SimpleCov::Result in the resultset cache From 6dafc25ad591d21a479c8e35b2659ca429c2f981 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 14 Feb 2017 14:51:55 -0500 Subject: [PATCH 06/10] Fix RuboCop offenses --- lib/simplecov/raw_coverage.rb | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/simplecov/raw_coverage.rb b/lib/simplecov/raw_coverage.rb index 6f6cba19..8824682c 100644 --- a/lib/simplecov/raw_coverage.rb +++ b/lib/simplecov/raw_coverage.rb @@ -1,6 +1,6 @@ module SimpleCov module RawCoverage - module_function + module_function # Merges multiple Coverage.result hashes def merge_results(*results) @@ -10,26 +10,28 @@ def merge_results(*results) end # Merges two Coverage.result hashes - def merge_resultsets(a, b) - (a.keys | b.keys).each_with_object({}) do |filename, merged| - result1 = a[filename] - result2 = b[filename] - merged[filename] = merge_file_coverage(result1, result2) + def merge_resultsets(result1, result2) + (result1.keys | result2.keys).each_with_object({}) do |filename, merged| + file1 = result1[filename] + file2 = result2[filename] + merged[filename] = merge_file_coverage(file1, file2) end end - def merge_file_coverage(a, b) - unless a && b - return (a || b).dup + def merge_file_coverage(file1, file2) + return (file1 || file2).dup unless file1 && file2 + + file1.zip(file2).map do |count1, count2| + merge_line_coverage(count1, count2) end + end - a.zip(b).map do |count1, count2| - sum = count1.to_i + count2.to_i - if sum.zero? && (count1.nil? || count2.nil?) - nil - else - sum - end + def merge_line_coverage(count1, count2) + sum = count1.to_i + count2.to_i + if sum.zero? && (count1.nil? || count2.nil?) + nil + else + sum end end end From 794cecb5cf56fac0f3b0951af89124d93e6bbed3 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Fri, 17 Feb 2017 14:59:41 -0500 Subject: [PATCH 07/10] Use map.with_index instead of zip for performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $ cat bench.rb require "benchmark/ips" a = Array.new(100) { 1 }.freeze b = Array.new(100) { 2 }.freeze Benchmark.ips do |x| x.report('lazy.zip.map.to_a') { a.lazy.zip(b).map {|l,r| l + r }.to_a } x.report('zip.map') { a.zip(b).map {|l,r| l + r } } x.report('map.with_index') { a.map.with_index {|i,index| i + b[index] } } x.report('each_with_index') do |times| new_array = [] i = 0 while i < times a.each_with_index {|i,index| new_array << i + b[index] } i = i + 1 end end x.compare! end $ ruby bench.rb Warming up -------------------------------------- lazy.zip.map.to_a 1.450k i/100ms zip.map 7.869k i/100ms map.with_index 10.748k i/100ms each_with_index 9.976k i/100ms Calculating ------------------------------------- lazy.zip.map.to_a 13.688k (±14.1%) i/s - 68.150k in 5.097768s zip.map 79.369k (± 8.6%) i/s - 393.450k in 5.005312s map.with_index 110.505k (± 4.0%) i/s - 558.896k in 5.067126s each_with_index 114.300k (± 4.2%) i/s - 578.608k in 5.071720s Comparison: each_with_index: 114300.5 i/s map.with_index: 110505.2 i/s - same-ish: difference falls within error zip.map: 79368.7 i/s - 1.44x slower lazy.zip.map.to_a: 13688.2 i/s - 8.35x slower --- lib/simplecov/raw_coverage.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/simplecov/raw_coverage.rb b/lib/simplecov/raw_coverage.rb index 8824682c..91e54545 100644 --- a/lib/simplecov/raw_coverage.rb +++ b/lib/simplecov/raw_coverage.rb @@ -21,7 +21,8 @@ def merge_resultsets(result1, result2) def merge_file_coverage(file1, file2) return (file1 || file2).dup unless file1 && file2 - file1.zip(file2).map do |count1, count2| + file1.map.with_index do |count1, index| + count2 = file2[index] merge_line_coverage(count1, count2) end end From fbebb996ee4d0bc5802e6533770dd38c56df2cd0 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Fri, 17 Feb 2017 15:05:57 -0500 Subject: [PATCH 08/10] Make frozen result merging spec more clear Now explicitly assert that no error is raised. --- spec/raw_coverage_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/raw_coverage_spec.rb b/spec/raw_coverage_spec.rb index 3674db17..74a6b90a 100644 --- a/spec/raw_coverage_spec.rb +++ b/spec/raw_coverage_spec.rb @@ -86,7 +86,9 @@ end it "can merge without exceptions" do - SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2) + expect { + SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2) + }.not_to raise_error end end end From e8643901bcd241f0e0eb530ffd327c44a82607c7 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 21 Feb 2017 10:56:11 -0500 Subject: [PATCH 09/10] Test merging frozen resultsets more thoroughly --- spec/raw_coverage_spec.rb | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/spec/raw_coverage_spec.rb b/spec/raw_coverage_spec.rb index 74a6b90a..a76d44c8 100644 --- a/spec/raw_coverage_spec.rb +++ b/spec/raw_coverage_spec.rb @@ -72,24 +72,21 @@ end end - describe "with frozen resultsets" do - before do - @resultset1 = { - source_fixture("sample.rb").freeze => [nil, 1, 1, 1, nil, nil, 1, 1, nil, nil].freeze, - source_fixture("app/models/user.rb").freeze => [nil, 1, 1, 1, nil, nil, 1, 0, nil, nil].freeze, - }.freeze - - @resultset2 = { - source_fixture("sample.rb").freeze => [1, nil, 1, 1, nil, nil, 1, 1, nil, nil].freeze, - source_fixture("app/models/user.rb").freeze => [nil, 1, 5, 1, nil, nil, 1, 0, nil, nil].freeze, - }.freeze - end - - it "can merge without exceptions" do - expect { - SimpleCov::RawCoverage.merge_results(@resultset1, @resultset2) - }.not_to raise_error - end + it "merges frozen resultsets" do + resultset1 = { + source_fixture("sample.rb").freeze => [nil, 1, 1, 1, nil, nil, 1, 1, nil, nil].freeze, + source_fixture("app/models/user.rb").freeze => [nil, 1, 1, 1, nil, nil, 1, 0, nil, nil].freeze, + }.freeze + + resultset2 = { + source_fixture("sample.rb").freeze => [1, nil, 1, 1, nil, nil, 1, 1, nil, nil].freeze, + }.freeze + + merged_result = SimpleCov::RawCoverage.merge_results(resultset1, resultset2) + expect(merged_result.keys).to eq(resultset1.keys) + expect(merged_result.values.map(&:frozen?)).to eq([false, false]) + expect(merged_result[source_fixture('sample.rb')]).to eq([1, 1, 2, 2, nil, nil, 2, 2, nil, nil]) + expect(merged_result[source_fixture('app/models/user.rb')]).to eq([nil, 1, 1, 1, nil, nil, 1, 0, nil, nil]) end end end From c7a9d82248c38a71416959ebfe4ae2e9c7842161 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 21 Feb 2017 11:22:52 -0500 Subject: [PATCH 10/10] Use double quotes to appease RuboCop --- spec/raw_coverage_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/raw_coverage_spec.rb b/spec/raw_coverage_spec.rb index a76d44c8..f7b6b0c5 100644 --- a/spec/raw_coverage_spec.rb +++ b/spec/raw_coverage_spec.rb @@ -85,8 +85,8 @@ merged_result = SimpleCov::RawCoverage.merge_results(resultset1, resultset2) expect(merged_result.keys).to eq(resultset1.keys) expect(merged_result.values.map(&:frozen?)).to eq([false, false]) - expect(merged_result[source_fixture('sample.rb')]).to eq([1, 1, 2, 2, nil, nil, 2, 2, nil, nil]) - expect(merged_result[source_fixture('app/models/user.rb')]).to eq([nil, 1, 1, 1, nil, nil, 1, 0, nil, nil]) + expect(merged_result[source_fixture("sample.rb")]).to eq([1, 1, 2, 2, nil, nil, 2, 2, nil, nil]) + expect(merged_result[source_fixture("app/models/user.rb")]).to eq([nil, 1, 1, 1, nil, nil, 1, 0, nil, nil]) end end end