Merge pull request #195 from danmayer/feature/relative_path_in_store

move store to use relative paths, this handles when paths are differe…

lib/coverband.rb

@@ -7,6 +7,7 @@
 require 'coverband/version'
 require 'coverband/at_exit'
 require 'coverband/configuration'
+require 'coverband/utils/file_path_helper'
 require 'coverband/adapters/base'
 require 'coverband/adapters/redis_store'
 require 'coverband/adapters/file_store'

lib/coverband/adapters/base.rb

@@ -3,6 +3,8 @@
 module Coverband
   module Adapters
     class Base
+      include Coverband::Utils::FilePathHelper
+
       def initialize
         @file_hash_cache = {}
       end
@@ -17,7 +19,7 @@ def clear!
       # and the tradeoff has always been acceptable
       def save_report(report)
         data = report.dup
-        merge_reports(data, get_report)
+        data = merge_reports(data, get_report)
         save_coverage(data)
       end
 
@@ -49,6 +51,7 @@ def file_hash(file)
       end
 
       def expand_report(report)
+        expanded = {}
         report_time = Time.now.to_i
         report.each_pair do |key, line_data|
           extended_data = {
@@ -57,8 +60,9 @@ def expand_report(report)
             'file_hash' => file_hash(key),
             'data' => line_data
           }
-          report[key] = extended_data
+          expanded[full_path_to_relative(key)] = extended_data
         end
+        expanded
       end
 
       def merge_reports(new_report, old_report)

lib/coverband/adapters/redis_store.rb

@@ -11,7 +11,7 @@ class RedisStore < Base
       # used to store data to redis. It is changed only when breaking changes to our
       # redis format are required.
       ###
-      REDIS_STORAGE_FORMAT_VERSION = 'coverband3_1'
+      REDIS_STORAGE_FORMAT_VERSION = 'coverband_3_2'
 
       def initialize(redis, opts = {})
         super()

lib/coverband/configuration.rb

@@ -121,7 +121,18 @@ def gem_paths
       Gem::PathSupport.new(ENV).path.select { |path| File.exist?(path) }
     end
 
-    SKIPPED_SETTINGS = %w[@s3_secret_access_key @store]
+    def current_root
+      File.expand_path(Coverband.configuration.root)
+    end
+
+    def all_root_paths
+      roots = Coverband.configuration.root_paths.dup
+      roots += Coverband.configuration.gem_paths.dup if Coverband.configuration.track_gems
+      roots << "#{Coverband.configuration.current_root}/"
+      roots
+    end
+
+    SKIPPED_SETTINGS = %w(@s3_secret_access_key @store)
     def to_h
       instance_variables
         .each_with_object('gem_paths': gem_paths) do |var, hash|

lib/coverband/reporters/base.rb

@@ -8,8 +8,10 @@ module Reporters
     ###
     class Base
       class << self
+        include Coverband::Utils::FilePathHelper
         def report(store, _options = {})
-          scov_style_report = get_current_scov_data_imp(store, root_paths)
+          all_roots = Coverband.configuration.all_root_paths
+          scov_style_report = get_current_scov_data_imp(store, all_roots)
 
           if Coverband.configuration.verbose
             msg = "report:\n #{scov_style_report.inspect}"
@@ -20,25 +22,14 @@ def report(store, _options = {})
 
         protected
 
-        def root_paths
-          roots = Coverband.configuration.root_paths
-          roots += Coverband.configuration.gem_paths if Coverband.configuration.track_gems
-          roots << "#{current_root}/"
-          roots
-        end
-
-        def current_root
-          File.expand_path(Coverband.configuration.root)
-        end
-
         def fix_file_names(report_hash, roots)
           if Coverband.configuration.verbose
             Coverband.configuration.logger.info "fixing root: #{roots.join(', ')}"
           end
 
           # normalize names across servers
           report_hash.each_with_object({}) do |(key, vals), fixed_report|
-            filename = filename_from_key(key, roots)
+            filename = relative_path_to_full(key, roots)
             fixed_report[filename] = if fixed_report.key?(filename) && fixed_report[filename]['data'] && vals['data']
                                        merged_data = merge_arrays(fixed_report[filename]['data'], vals['data'])
                                        vals['data'] = merged_data
@@ -64,40 +55,6 @@ def merge_arrays(first, second)
           merged
         end
 
-        ###
-        # filename_from_key code takes:
-        # key: which is a full path the same as reported by Coverage
-        # roots: if a collection of all possible full app paths
-        #    EX: [Coverband.configuration.root_paths, "#{current_root}/"]
-        # The LAST item should be the current file system root
-        # it expands that expands and adds a '/' as that isn't there from Dir.pwd
-        #
-        # NOTEs on configuration.root_paths usage
-        # strings: matching is pretty simple for full string paths
-        # regex: to get regex to work for changing deploy directories
-        #        the regex must be double escaped in double quotes
-        #          (if using \d for example)
-        #        or use single qoutes
-        #        example: '/box/apps/app_name/releases/\d+/'
-        #        example: '/var/local/company/company.d/[0-9]*/'
-        ###
-        def filename_from_key(key, roots)
-          relative_filename = key
-          local_filename = relative_filename
-          roots.each do |root|
-            relative_filename = relative_filename.gsub(/^#{root}/, './')
-          end
-          # the filename for our reports is expected to be a full path.
-          # roots.last should be roots << current_root}/
-          # a fully expanded path of config.root
-          # filename = filename.gsub('./', roots.last)
-          # above only works for app files
-          # we need to rethink some of this logic
-          # gems aren't at project root and can have multiple locations
-          local_root = roots.find { |root| File.exist?(relative_filename.gsub('./', root)) }
-          local_root ? relative_filename.gsub('./', local_root) : local_filename
-        end
-
         ###
         # why do we need to merge covered files data?
         # basically because paths on machines or deployed hosts could be different, so

lib/coverband/reporters/html_report.rb

@@ -11,7 +11,7 @@ def self.report(store, options = {})
         base_path = options.fetch(:base_path) { nil }
 
         # list all files, even if not tracked by Coverband (0% coverage)
-        tracked_glob = "#{current_root}/{app,lib,config}/**/*.{rb}"
+        tracked_glob = "#{Coverband.configuration.current_root}/{app,lib,config}/**/*.{rb}"
         report_files = Coverband::Utils::Result.add_not_loaded_files(scov_style_report, tracked_glob)
         # apply coverband filters
         filtered_report_files = {}

lib/coverband/utils/file_path_helper.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+####
+# Helper functions for shared logic related to file path manipulation
+####
+module Coverband
+  module Utils
+    module FilePathHelper
+      ###
+      # Takes a full path and converts to a relative path
+      ###
+      def full_path_to_relative(full_path)
+        relative_filename = full_path
+        Coverband.configuration.all_root_paths.each do |root|
+          relative_filename = relative_filename.gsub(/^#{root}/, './')
+          # once we have a relative path break out of the loop
+          break if relative_filename.start_with? './'
+        end
+        relative_filename
+      end
+
+      ###
+      # relative_path_to_full code takes:
+      # relative_path: which is a full path the same as reported by Coverage
+      # roots: if a collection of all possible full app paths
+      #    EX: [Coverband.configuration.root_paths, "#{current_root}/"]
+      # The LAST item should be the current file system root
+      # it expands that expands and adds a '/' as that isn't there from Dir.pwd
+      #
+      # NOTEs on configuration.root_paths usage
+      # strings: matching is pretty simple for full string paths
+      # regex: to get regex to work for changing deploy directories
+      #        the regex must be double escaped in double quotes
+      #          (if using \d for example)
+      #        or use single qoutes
+      #        example: '/box/apps/app_name/releases/\d+/'
+      #        example: '/var/local/company/company.d/[0-9]*/'
+      ###
+      def relative_path_to_full(relative_path, roots)
+        relative_filename = relative_path
+        local_filename = relative_filename
+        roots.each do |root|
+          relative_filename = relative_filename.gsub(/^#{root}/, './')
+        end
+        # the filename for our reports is expected to be a full path.
+        # roots.last should be roots << current_root}/
+        # a fully expanded path of config.root
+        # filename = filename.gsub('./', roots.last)
+        # above only works for app files
+        # we need to rethink some of this logic
+        # gems aren't at project root and can have multiple locations
+        local_root = roots.find { |root| File.exist?(relative_filename.gsub('./', root)) }
+        local_root ? relative_filename.gsub('./', local_root) : local_filename
+      end
+    end
+  end
+end

test/benchmarks/benchmark.rake

@@ -176,6 +176,10 @@ namespace :benchmarks do
         @file_hash_cache[file] = Digest::MD5.file(__FILE__).hexdigest
       end
     end
+
+    def store.full_path_to_relative(file)
+      file
+    end
   end
 
   def reporting_speed

test/unit/configuration_test.rb

@@ -16,22 +16,32 @@ def setup
     end
   end
 
-  test 'defaults ' do
+  test 'defaults' do
     coverband = Coverband::Collectors::Coverage.instance.reset_instance
     assert_equal ['vendor', 'internal:prelude', 'schema.rb'], coverband.instance_variable_get('@ignore_patterns')
   end
 
-  test 'gem_paths ' do
+  test 'gem_paths' do
     Coverband::Collectors::Coverage.instance.reset_instance
     assert Coverband.configuration.gem_paths.first != nil
   end
 
-  test 'groups ' do
+  test 'groups' do
     Coverband::Collectors::Coverage.instance.reset_instance
     Coverband.configuration.track_gems = true
     assert_equal %w(App Gems), Coverband.configuration.groups.keys
   end
 
+  test 'all_root_paths' do
+    Coverband::Collectors::Coverage.instance.reset_instance
+    current_paths = Coverband.configuration.root_paths.dup
+    # verify previous bug fix
+    # it would extend the root_paths instance variable on each invokation
+    Coverband.configuration.all_root_paths
+    all_roots = Coverband.configuration.all_root_paths
+    assert_equal current_paths, Coverband.configuration.root_paths
+  end
+
   test 's3 options' do
     Coverband::Collectors::Coverage.instance.reset_instance
     Coverband.configure do |config|

test/unit/full_stack_test.rb

@@ -6,6 +6,7 @@
 class FullStackTest < Minitest::Test
   REDIS_STORAGE_FORMAT_VERSION = Coverband::Adapters::RedisStore::REDIS_STORAGE_FORMAT_VERSION
   TEST_RACK_APP = '../fake_app/basic_rack.rb'
+  RELATIVE_FILE = './fake_app/basic_rack.rb'
 
   def setup
     super
@@ -15,6 +16,7 @@ def setup
       config.store = Coverband::Adapters::RedisStore.new(Redis.new)
       config.s3_bucket = nil
       config.background_reporting_enabled = false
+      config.root_paths = ["#{File.expand_path('../', File.dirname(__FILE__))}/"]
     end
     Coverband.configuration.store.clear!
     Coverband.start
@@ -29,13 +31,13 @@ def setup
     assert_equal 'Hello Rack!', results.last
     sleep(0.1)
     expected = [nil, nil, 1, nil, 1, 1, 1, nil, nil]
-    assert_equal expected, Coverband.configuration.store.coverage[@rack_file]['data']
+    assert_equal expected, Coverband.configuration.store.coverage[RELATIVE_FILE]['data']
 
     # additional calls increase count by 1
     middleware.call(request)
     sleep(0.1)
     expected = [nil, nil, 1, nil, 1, 1, 2, nil, nil]
-    assert_equal expected, Coverband.configuration.store.coverage[@rack_file]['data']
+    assert_equal expected, Coverband.configuration.store.coverage[RELATIVE_FILE]['data']
   end
 
   test 'call app with gem tracking' do

test/unit/reports_base_test.rb

@@ -3,7 +3,7 @@
 require File.expand_path('../test_helper', File.dirname(__FILE__))
 
 class ReportsBaseTest < Minitest::Test
-  test 'filename_from_key fix filename from a key with a swappable path' do
+  test 'relative_path_to_full fix filename from a key with a swappable path' do
     Coverband.configure do |config|
       config.reporter          = 'std_out'
       config.root              = '/full/remote_app/path'
@@ -16,10 +16,10 @@ class ReportsBaseTest < Minitest::Test
     expected_path = '/full/remote_app/path/is/a/path.rb'
     File.expects(:exist?).with(key).returns(false)
     File.expects(:exist?).with(expected_path).returns(true)
-    assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
+    assert_equal expected_path, Coverband::Reporters::Base.send(:relative_path_to_full, key, roots)
   end
 
-  test 'filename_from_key fix filename a changing deploy path with quotes' do
+  test 'relative_path_to_full fix filename a changing deploy path with quotes' do
     Coverband.configure do |config|
       config.reporter          = 'std_out'
       config.root              = '/full/remote_app/path'
@@ -30,14 +30,14 @@ class ReportsBaseTest < Minitest::Test
     roots = ["/box/apps/app_name/releases/\\d+/", '/full/remote_app/path/']
     File.expects(:exist?).with('/box/apps/app_name/releases/\\d+/app/models/user.rb').returns(false)
     File.expects(:exist?).with(expected_path).returns(true)
-    assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
+    assert_equal expected_path, Coverband::Reporters::Base.send(:relative_path_to_full, key, roots)
     File.expects(:exist?).with('/box/apps/app_name/releases/\\d+/app/models/user.rb').returns(false)
     File.expects(:exist?).with(expected_path).returns(true)
     roots = ['/box/apps/app_name/releases/\d+/', '/full/remote_app/path/']
-    assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
+    assert_equal expected_path, Coverband::Reporters::Base.send(:relative_path_to_full, key, roots)
   end
 
-  test 'filename_from_key fix filename a changing deploy path real world examples' do
+  test 'relative_path_to_full fix filename a changing deploy path real world examples' do
     current_app_root = '/var/local/company/company.d/79'
     Coverband.configure do |config|
       config.reporter          = 'std_out'
@@ -50,14 +50,14 @@ class ReportsBaseTest < Minitest::Test
     File.expects(:exist?).with('/var/local/company/company.d/[0-9]*/app/controllers/dashboard_controller.rb').returns(false)
     File.expects(:exist?).with(expected_path).returns(true)
     roots = ['/var/local/company/company.d/[0-9]*/', "#{current_app_root}/"]
-    assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
+    assert_equal expected_path, Coverband::Reporters::Base.send(:relative_path_to_full, key, roots)
     File.expects(:exist?).with('/var/local/company/company.d/[0-9]*/app/controllers/dashboard_controller.rb').returns(false)
     File.expects(:exist?).with(expected_path).returns(true)
     roots = ["/var/local/company/company.d/[0-9]*/", "#{current_app_root}/"]
-    assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
+    assert_equal expected_path, Coverband::Reporters::Base.send(:relative_path_to_full, key, roots)
   end
 
-  test 'filename_from_key leave filename from a key with a local path' do
+  test 'relative_path_to_full leave filename from a key with a local path' do
     Coverband.configure do |config|
       config.reporter          = 'std_out'
       config.root              = '/full/remote_app/path'
@@ -68,7 +68,7 @@ class ReportsBaseTest < Minitest::Test
     roots = ['/app/', '/full/remote_app/path/']
 
     expected_path = '/full/remote_app/path/is/a/path.rb'
-    assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
+    assert_equal expected_path, Coverband::Reporters::Base.send(:relative_path_to_full, key, roots)
   end
 
   test '#merge_arrays basic merge preserves order and counts' do

test/unit/reports_console_test.rb

@@ -20,13 +20,14 @@ def setup
     end
     Coverband.configuration.logger.stubs('info')
     mock_file_hash
-    Coverband::Reporters::ConsoleReport
-      .expects(:current_root)
-      .returns('app_path')
+    Coverband.configuration
+             .expects(:current_root)
+             .at_least_once
+             .returns('app_path')
     @store.send(:save_report, basic_coverage)
 
     report = Coverband::Reporters::ConsoleReport.report(@store)
-    expected = { 'app_path/dog.rb' => [0, 1, 2] }
+    expected = { './dog.rb' => [0, 1, 2] }
     assert_equal(expected.keys, report.keys)
   end
 end