fix results collation

Author: tycooon

Gemfile

@@ -2,7 +2,6 @@
 
 source "https://rubygems.org"
 
-# rubocop:disable Bundler/DuplicatedGem
 case ENV["SIMPLECOV_HTML_MODE"]
 when "local"
   # Use local copy of simplecov-html in development when checked out
@@ -11,8 +10,6 @@ when "github"
   # Use development version of html formatter from github
   gem "simplecov-html", github: "simplecov-ruby/simplecov-html"
 end
-# rubocop:enable Bundler/DuplicatedGem
-
 group :development do
   gem "apparition", "~> 0.6.0"
   gem "aruba", "~> 1.0"

Gemfile.lock

@@ -179,4 +179,4 @@ DEPENDENCIES
   webrick
 
 BUNDLED WITH
-   2.2.8
+   2.2.16

features/method_coverage.feature

@@ -16,6 +16,7 @@ module FilesCombiner
       #
       def combine(cov_a, cov_b)
         combination = {}
+
         combination[:lines] = Combine.combine(LinesCombiner, cov_a[:lines], cov_b[:lines])
 
         if SimpleCov.branch_coverage? # rubocop:disable Style/IfUnlessModifier

lib/simplecov/combine/files_combiner.rb

@@ -73,18 +73,14 @@ def to_hash
     # Loads a SimpleCov::Result#to_hash dump
     def self.from_hash(hash)
       SimpleCov::ResultSerialization.deserialize(hash)
-      # hash.map do |command_name, data|
-      #   new(data.fetch("coverage"), command_name: command_name, created_at: Time.at(data["timestamp"]))
-      # end
     end
 
   private
 
-    # TODO[@tycooon]: remove?
-    # def coverage
-    #   keys = original_result.keys & filenames
-    #   Hash[keys.zip(original_result.values_at(*keys))]
-    # end
+    def coverage
+      keys = original_result.keys & filenames
+      Hash[keys.zip(original_result.values_at(*keys))]
+    end
 
     # Applies all configured SimpleCov filters on this result's source files
     def filter!

lib/simplecov/result.rb

@@ -31,20 +31,14 @@ def merge_results(*file_paths, ignore_timeout: false)
         # In big CI setups you might deal with 100s of CI jobs and each one producing Megabytes
         # of data. Reading them all in easily produces Gigabytes of memory consumption which
         # we want to avoid.
-        #
-        # For similar reasons a SimpleCov::Result is only created in the end as that'd create
-        # even more data especially when it also reads in all source files.
-        initial_memo = valid_results(file_paths.shift, ignore_timeout: ignore_timeout)
-
-        command_names, coverage = file_paths.reduce(initial_memo) do |memo, file_path|
-          merge_coverage(memo, valid_results(file_path, ignore_timeout: ignore_timeout))
-        end
 
-        create_result(command_names, coverage)
+        results = file_paths.map { |path| valid_results(path, ignore_timeout: ignore_timeout) }
+        merge_coverage(results)
       end
 
       def valid_results(file_path, ignore_timeout: false)
-        results = parse_file(file_path)
+        raw_results = parse_file(file_path)
+        results = Result.from_hash(raw_results)
         merge_valid_results(results, ignore_timeout: ignore_timeout)
       end
 
@@ -72,42 +66,25 @@ def parse_json(content)
       end
 
       def merge_valid_results(results, ignore_timeout: false)
-        results = results.select { |_command_name, data| within_merge_timeout?(data) } unless ignore_timeout
-
-        command_plus_coverage = results.map do |command_name, data|
-          [[command_name], adapt_result(data.fetch("coverage"))]
-        end
-
-        # one file itself _might_ include multiple test runs
-        merge_coverage(*command_plus_coverage)
+        results = results.select { |x| within_merge_timeout?(x) } unless ignore_timeout
+        merge_coverage(results)
       end
 
-      def within_merge_timeout?(data)
-        time_since_result_creation(data) < SimpleCov.merge_timeout
+      def within_merge_timeout?(result)
+        Time.now - result.created_at < SimpleCov.merge_timeout
       end
 
-      def time_since_result_creation(data)
-        Time.now - Time.at(data.fetch("timestamp"))
-      end
-
-      def create_result(command_names, coverage)
-        return nil unless coverage
-
-        command_name = command_names.reject(&:empty?).sort.join(", ")
-        SimpleCov::Result.new(coverage, command_name: command_name)
-      end
+      def merge_coverage(results)
+        results = results.compact
 
-      def merge_coverage(*results)
-        return [[""], nil] if results.empty?
+        return nil if results.size.zero?
         return results.first if results.size == 1
 
-        results.reduce do |(memo_command, memo_coverage), (command, coverage)|
-          # timestamp is dropped here, which is intentional (we merge it, it gets a new time stamp as of now)
-          merged_coverage = Combine.combine(Combine::ResultsCombiner, memo_coverage, coverage)
-          merged_command = memo_command + command
-
-          [merged_command, merged_coverage]
-        end
+        parsed_results = results.map(&:original_result)
+        combined_result = SimpleCov::Combine::ResultsCombiner.combine(*parsed_results)
+        result = SimpleCov::Result.new(combined_result)
+        result.command_name = results.map(&:command_name).reject(&:empty?).sort.join(", ")
+        result
       end
 
       #
@@ -118,9 +95,8 @@ def merged_result
         # conceptually this is just doing `merge_results(resultset_path)`
         # it's more involved to make syre `synchronize_resultset` is only used around reading
         resultset_hash = read_resultset
-        command_names, coverage = merge_valid_results(resultset_hash)
-
-        create_result(command_names, coverage)
+        results = Result.from_hash(resultset_hash)
+        merge_valid_results(results)
       end
 
       def read_resultset
@@ -164,31 +140,6 @@ def synchronize_resultset
           @resultset_locked = false
         end
       end
-
-      # We changed the format of the raw result data in simplecov, as people are likely
-      # to have "old" resultsets lying around (but not too old so that they're still
-      # considered we can adapt them).
-      # See https://github.com/simplecov-ruby/simplecov/pull/824#issuecomment-576049747
-      def adapt_result(result)
-        if pre_simplecov_0_18_result?(result)
-          adapt_pre_simplecov_0_18_result(result)
-        else
-          result
-        end
-      end
-
-      # pre 0.18 coverage data pointed from file directly to an array of line coverage
-      def pre_simplecov_0_18_result?(result)
-        _key, data = result.first
-
-        data.is_a?(Array)
-      end
-
-      def adapt_pre_simplecov_0_18_result(result)
-        result.transform_values do |line_coverage_data|
-          {"lines" => line_coverage_data}
-        end
-      end
     end
   end
 end

lib/simplecov/result_merger.rb

@@ -123,7 +123,7 @@
         it "has the result stored" do
           SimpleCov::ResultMerger.merge_and_store(resultset1_path, resultset2_path)
 
-          expect_resultset_1_and_2_merged(SimpleCov::ResultMerger.read_resultset)
+          expect_resultset_1_and_2_merged(SimpleCov::ResultMerger.merged_result.to_hash)
         end
       end
 

spec/result_merger_spec.rb

@@ -207,15 +207,18 @@
         "result1, result2" => {
           "coverage" => {
             source_fixture("sample.rb") => {
-              "lines" => [1, 1, 2, 2, nil, nil, 2, 2, nil, nil]
+              lines: [1, 1, 2, 2, nil, nil, 2, 2, nil, nil]
             }
           }
         }
       }
     end
 
     let(:collated) do
-      JSON.parse(File.read(resultset_path)).transform_values { |v| v.reject { |k| k == "timestamp" } }
+      JSON.parse(File.read(resultset_path)).transform_values do |data|
+        data["coverage"].values.first.transform_keys!(&:to_sym)
+        data.reject { |k| k == "timestamp" }
+      end
     end
 
     context "when no files to be merged" do