Merge pull request #558 from aroben/merge-without-extend

Rewrite merge helpers to work with frozen objects

Author: PragTob

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/raw_coverage"
 require "simplecov/result_merger"
 require "simplecov/command_guesser"
 require "simplecov/version"

lib/simplecov/merge_helpers.rb

@@ -0,0 +1,39 @@
+module SimpleCov
+  module RawCoverage
+  module_function
+
+    # Merges multiple Coverage.result hashes
+    def merge_results(*results)
+      results.reduce({}) do |result, merged|
+        merge_resultsets(result, merged)
+      end
+    end
+
+    # Merges two Coverage.result hashes
+    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(file1, file2)
+      return (file1 || file2).dup unless file1 && file2
+
+      file1.map.with_index do |count1, index|
+        count2 = file2[index]
+        merge_line_coverage(count1, count2)
+      end
+    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
+end

lib/simplecov/raw_coverage.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)

lib/simplecov/result.rb

@@ -54,20 +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 = {}
-        results.each do |result|
-          merged = result.original_result.merge_resultset(merged)
-        end
-        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

lib/simplecov/result_merger.rb

@@ -0,0 +1,92 @@
+require "helper"
+
+if SimpleCov.usable?
+  describe SimpleCov::RawCoverage do
+    describe "with two faked coverage resultsets" do
+      before do
+        @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
+          source_fixture("three.rb") => [nil, 1, 1],
+        }
+
+        @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
+          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, @resultset3)
+        end
+
+        it "has proper results for sample.rb" do
+          expect(subject[source_fixture("sample.rb")]).to eq([1, 1, 2, 2, nil, nil, 2, 2, nil, nil])
+        end
+
+        it "has proper results for user.rb" do
+          expect(subject[source_fixture("app/models/user.rb")]).to eq([nil, 2, 6, 2, nil, nil, 2, 0, nil, nil])
+        end
+
+        it "has proper results for sample_controller.rb" do
+          expect(subject[source_fixture("app/controllers/sample_controller.rb")]).to eq([nil, 4, 2, 1, nil, nil, 2, 0, nil, nil])
+        end
+
+        it "has proper results for resultset1.rb" do
+          expect(subject[source_fixture("resultset1.rb")]).to eq([1, 1, 1, 1])
+        end
+
+        it "has proper results for resultset2.rb" do
+          expect(subject[source_fixture("resultset2.rb")]).to eq([nil, 1, 1, nil])
+        end
+
+        it "has proper results for parallel_tests.rb" do
+          expect(subject[source_fixture("parallel_tests.rb")]).to eq([nil, nil, nil, 0])
+        end
+
+        it "has proper results for conditionally_loaded_1.rb" do
+          expect(subject[source_fixture("conditionally_loaded_1.rb")]).to eq([nil, 0, 1])
+        end
+
+        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
+
+    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

spec/raw_coverage_spec.rb

@@ -1,7 +1,7 @@
 require "helper"
 
 if SimpleCov.usable?
-  describe "merge helpers" do
+  describe SimpleCov::ResultMerger 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],
@@ -24,44 +24,6 @@
         }
       end
 
-      context "a merge" do
-        subject do
-          @resultset1.merge_resultset(@resultset2)
-        end
-
-        it "has proper results for sample.rb" do
-          expect(subject[source_fixture("sample.rb")]).to eq([1, 1, 2, 2, nil, nil, 2, 2, nil, nil])
-        end
-
-        it "has proper results for user.rb" do
-          expect(subject[source_fixture("app/models/user.rb")]).to eq([nil, 2, 6, 2, nil, nil, 2, 0, nil, nil])
-        end
-
-        it "has proper results for sample_controller.rb" do
-          expect(subject[source_fixture("app/controllers/sample_controller.rb")]).to eq([nil, 4, 2, 1, nil, nil, 2, 0, nil, nil])
-        end
-
-        it "has proper results for resultset1.rb" do
-          expect(subject[source_fixture("resultset1.rb")]).to eq([1, 1, 1, 1])
-        end
-
-        it "has proper results for resultset2.rb" do
-          expect(subject[source_fixture("resultset2.rb")]).to eq([nil, 1, 1, nil])
-        end
-
-        it "has proper results for parallel_tests.rb" do
-          expect(subject[source_fixture("parallel_tests.rb")]).to eq([nil, nil, nil, 0])
-        end
-
-        it "has proper results for conditionally_loaded_1.rb" do
-          expect(subject[source_fixture("conditionally_loaded_1.rb")]).to eq([nil, 0, 1])
-        end
-
-        it "has proper results for conditionally_loaded_2.rb" do
-          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 "" }