Skip to content

Commit

Permalink
improve tests and docs for filename_from_key
Browse files Browse the repository at this point in the history
  • Loading branch information
Dan Mayer committed Feb 9, 2019
1 parent 3130012 commit c10da72
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
21 changes: 19 additions & 2 deletions lib/coverband/reporters/base.rb
Expand Up @@ -61,14 +61,31 @@ 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

This comment has been minimized.

Copy link
@gingerlime

gingerlime Feb 10, 2019

this sounds a bit dangerous or confusing to me... Our styleguide is to always use " for example.

Perhaps allow passing a real Regexp if you want to match a regex, or a String if you do a simple match? Then the code can become even simpler, e.g.

# this substitutes either a simple string or a regex, based on whatever is being passed
filename.gsub!(root, './')

As a caller, I can define, e.g.

config.root_paths = ["/app", /^var.*company\.d\/\d+/]

This comment has been minimized.

Copy link
@danmayer

danmayer Feb 10, 2019

Owner

yeah our styleguide would enforce the single quotes as well, but since Ruby doesn't it can be a bit weird... I commented back on the ticket but I don't think this is related tot he bug and it would break back-compat so I don' think I will address it unless we show it to be problematic.

# (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)
filename = key
roots.each do |root|
filename = filename.gsub(/^#{root}/, './')
end
# the filename for SimpleCov is expected to be a full path.
# The filename for Coverage report is expected to be a full LOCAL path.
# roots.last should be roots << current_root}/

This comment has been minimized.

Copy link
@gingerlime

gingerlime Feb 10, 2019

roots << "#{current_root}/", right?

This comment has been minimized.

Copy link
@danmayer

danmayer Feb 10, 2019

Owner

thanks cleaned up the comment

# a fully expanded path of config.root
# a fully expanded path of the file on the current runtime system
filename = filename.gsub('./', roots.last)
filename
end
Expand Down
25 changes: 13 additions & 12 deletions test/unit/reports_base_test.rb
Expand Up @@ -17,33 +17,34 @@ class ReportsBaseTest < Minitest::Test
assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
end

test 'filename_from_key fix filename a changing deploy path with double quotes' do
test 'filename_from_key fix filename a changing deploy path with quotes' do
Coverband.configure do |config|
config.reporter = 'std_out'
config.root = '/full/remote_app/path'
end

expected_path = '/full/remote_app/path/app/models/user.rb'
key = '/box/apps/app_name/releases/20140725203539/app/models/user.rb'
# the code takes config.root expands and adds a '/' for the final path in roots
# note to get regex to work for changing deploy directories it must be double escaped in double quotes or use single qoutes
roots = ['/box/apps/app_name/releases/\\d+/', '/full/remote_app/path/']
roots = ["/box/apps/app_name/releases/\\d+/", '/full/remote_app/path/']
assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)

expected_path = '/full/remote_app/path/app/models/user.rb'
roots = ['/box/apps/app_name/releases/\d+/', '/full/remote_app/path/']
assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
end

test 'filename_from_key fix filename a changing deploy path with single quotes' do
test 'filename_from_key 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'
config.root = '/full/remote_app/path'
config.root = current_app_root
end

key = '/box/apps/app_name/releases/20140725203539/app/models/user.rb'
# the code takes config.root expands and adds a '/' for the final path in roots
# note to get regex to work for changing deploy directories it must be double escaped in double quotes or use single qoutes
roots = ['/box/apps/app_name/releases/\d+/', '/full/remote_app/path/']
expected_path = '/var/local/company/company.d/79/app/controllers/dashboard_controller.rb'
key = '/var/local/company/company.d/78/app/controllers/dashboard_controller.rb'

expected_path = '/full/remote_app/path/app/models/user.rb'
roots = ['/var/local/company/company.d/[0-9]*/', "#{current_app_root}/"]
assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
roots = ["/var/local/company/company.d/[0-9]*/", "#{current_app_root}/"]
assert_equal expected_path, Coverband::Reporters::Base.send(:filename_from_key, key, roots)
end

Expand Down

0 comments on commit c10da72

Please sign in to comment.