Separate out file name format and remove first slash

Author: Kevin Paulisse

lib/octocatalog-diff/catalog.rb

@@ -209,6 +209,22 @@ def validate_references
 
     private
 
+    # Private method: Format the name of the source file and line number, based on compilation directory and
+    # other settings. This is used by format_missing_references.
+    # @param source_file [String] Raw source file name from catalog
+    # @param line_number [Fixnum] Line number from catalog
+    # @return [String] Formatted source file
+    def format_source_file_line(source_file, line_number)
+      return '' if source_file.nil? || source_file.empty?
+      filename = if compilation_dir && source_file.start_with?(compilation_dir)
+        stripped_file = source_file[compilation_dir.length..-1]
+        stripped_file.start_with?('/') ? stripped_file[1..-1] : stripped_file
+      else
+        source_file
+      end
+      "(#{filename}:#{line_number})"
+    end
+
     # Private method: Format the missing references into human-readable text
     # Error message will look like this:
     # ---
@@ -228,14 +244,7 @@ def format_missing_references(missing)
         # separate error message per element of that array. This allows the total number
         # of errors to be correct.
         src_ref = "#{obj[:source]['type'].downcase}[#{obj[:source]['title']}]"
-        src_file = if obj[:source]['file'].nil? || obj[:source]['file'].empty?
-          ''
-        elsif compilation_dir && obj[:source]['file'].start_with?(compilation_dir)
-          "(#{obj[:source]['file'][compilation_dir.length..-1]}:#{obj[:source]['line']})"
-        else
-          "(#{obj[:source]['file']}:#{obj[:source]['line']})"
-        end
-
+        src_file = format_source_file_line(obj[:source]['file'], obj[:source]['line'])
         target_val = obj[:target_value].is_a?(Array) ? obj[:target_value] : [obj[:target_value]]
         target_val.map { |tv| "#{src_ref}#{src_file} -> #{obj[:target_type].downcase}[#{tv}]" }
       end.flatten

spec/octocatalog-diff/integration/reference_validation_spec.rb

@@ -106,10 +106,10 @@ def self.catalog_contains_resource(result, type, title)
     # rubocop:disable Metrics/LineLength
     it 'should have formatted error messages' do
       msg = @result.exception.message
-      expect(msg).to match(%r{exec\[subscribe caller 1\]\(/environments/production/modules/test/manifests/subscribe_callers.pp:(2|5)\) -> subscribe\[Exec\[subscribe target\]\]})
-      expect(msg).to match(%r{exec\[subscribe caller 2\]\(/environments/production/modules/test/manifests/subscribe_callers.pp:(7|13)\) -> subscribe\[Exec\[subscribe target\]\]})
-      expect(msg).to match(%r{exec\[subscribe caller 2\]\(/environments/production/modules/test/manifests/subscribe_callers.pp:(7|13)\) -> subscribe\[Exec\[subscribe target 2\]\]})
-      expect(msg).to match(%r{exec\[subscribe caller 3\]\(/environments/production/modules/test/manifests/subscribe_callers.pp:(15|21)\) -> subscribe\[Exec\[subscribe target\]\]})
+      expect(msg).to match(%r{exec\[subscribe caller 1\]\(modules/test/manifests/subscribe_callers.pp:(2|5)\) -> subscribe\[Exec\[subscribe target\]\]})
+      expect(msg).to match(%r{exec\[subscribe caller 2\]\(modules/test/manifests/subscribe_callers.pp:(7|13)\) -> subscribe\[Exec\[subscribe target\]\]})
+      expect(msg).to match(%r{exec\[subscribe caller 2\]\(modules/test/manifests/subscribe_callers.pp:(7|13)\) -> subscribe\[Exec\[subscribe target 2\]\]})
+      expect(msg).to match(%r{exec\[subscribe caller 3\]\(modules/test/manifests/subscribe_callers.pp:(15|21)\) -> subscribe\[Exec\[subscribe target\]\]})
       expect(msg).not_to match(/exec\[subscribe caller 3\].+subscribe\[Exec\[subscribe caller 1\]\]/)
     end
     # rubocop:enable Metrics/LineLength
@@ -131,7 +131,7 @@ def self.catalog_contains_resource(result, type, title)
     # rubocop:disable Metrics/LineLength
     it 'should have formatted error messages' do
       msg = @result.exception.message
-      expect(msg).to match(%r{Catalog has broken reference: exec\[before caller\]\(/environments/production/modules/test/manifests/before_callers.pp:(2|5)\) -> before\[Exec\[before target\]\]})
+      expect(msg).to match(%r{Catalog has broken reference: exec\[before caller\]\(modules/test/manifests/before_callers.pp:(2|5)\) -> before\[Exec\[before target\]\]})
     end
     # rubocop:enable Metrics/LineLength
   end
@@ -152,7 +152,7 @@ def self.catalog_contains_resource(result, type, title)
     # rubocop:disable Metrics/LineLength
     it 'should have formatted error messages' do
       msg = @result.exception.message
-      expect(msg).to match(%r{exec\[notify caller\]\(/environments/production/modules/test/manifests/notify_callers.pp:(2|4)\) -> notify\[Test::Foo::Bar\[notify target\]\]})
+      expect(msg).to match(%r{exec\[notify caller\]\(modules/test/manifests/notify_callers.pp:(2|4)\) -> notify\[Test::Foo::Bar\[notify target\]\]})
     end
     # rubocop:enable Metrics/LineLength
   end
@@ -173,9 +173,9 @@ def self.catalog_contains_resource(result, type, title)
     # rubocop:disable Metrics/LineLength
     it 'should have formatted error messages' do
       msg = @result.exception.message
-      expect(msg).to match(%r{exec\[require caller\]\(/environments/production/modules/test/manifests/require_callers.pp:(2|5)\) -> require\[Exec\[require target\]\]})
-      expect(msg).to match(%r{exec\[require caller 3\]\(/environments/production/modules/test/manifests/require_callers.pp:(12|18)\) -> require\[Exec\[require target\]\]})
-      expect(msg).to match(%r{exec\[require caller 4\]\(/environments/production/modules/test/manifests/require_callers.pp:(12|18)\) -> require\[Exec\[require target\]\]})
+      expect(msg).to match(%r{exec\[require caller\]\(modules/test/manifests/require_callers.pp:(2|5)\) -> require\[Exec\[require target\]\]})
+      expect(msg).to match(%r{exec\[require caller 3\]\(modules/test/manifests/require_callers.pp:(12|18)\) -> require\[Exec\[require target\]\]})
+      expect(msg).to match(%r{exec\[require caller 4\]\(modules/test/manifests/require_callers.pp:(12|18)\) -> require\[Exec\[require target\]\]})
       expect(msg).not_to match(/exec\[require caller 2\]/)
       expect(msg).not_to match(/-> require\[Exec\[require caller\]\]/)
     end
@@ -235,10 +235,10 @@ def self.catalog_contains_resource(result, type, title)
     # rubocop:disable Metrics/LineLength
     it 'should have formatted error messages' do
       msg = @result.exception.message
-      expect(msg).to match(%r{exec\[before alias caller\]\(/environments/production/modules/test/manifests/alias_callers.pp:(2|5)\) -> before\[Exec\[before alias target\]\]})
-      expect(msg).to match(%r{exec\[notify alias caller\]\(/environments/production/modules/test/manifests/alias_callers.pp:(7|10)\) -> before\[Exec\[notify alias target\]\]})
-      expect(msg).to match(%r{exec\[require alias caller\]\(/environments/production/modules/test/manifests/alias_callers.pp:(12|15)\) -> before\[Exec\[require alias target\]\]})
-      expect(msg).to match(%r{exec\[subscribe alias caller\]\(/environments/production/modules/test/manifests/alias_callers.pp:(17|20)\) -> before\[Exec\[subscribe alias target\]\]})
+      expect(msg).to match(%r{exec\[before alias caller\]\(modules/test/manifests/alias_callers.pp:(2|5)\) -> before\[Exec\[before alias target\]\]})
+      expect(msg).to match(%r{exec\[notify alias caller\]\(modules/test/manifests/alias_callers.pp:(7|10)\) -> before\[Exec\[notify alias target\]\]})
+      expect(msg).to match(%r{exec\[require alias caller\]\(modules/test/manifests/alias_callers.pp:(12|15)\) -> before\[Exec\[require alias target\]\]})
+      expect(msg).to match(%r{exec\[subscribe alias caller\]\(modules/test/manifests/alias_callers.pp:(17|20)\) -> before\[Exec\[subscribe alias target\]\]})
     end
     # rubocop:enable Metrics/LineLength
   end

spec/octocatalog-diff/tests/catalog_spec.rb

@@ -473,11 +473,11 @@
       catalog = OctocatalogDiff::Catalog.new(opts)
       catalog.compilation_dir = '/var/folders/dw/5ftmkqk972j_kw2fdjyzdqdw0000gn/T/d20161223-46780-x10xaf/environments/production'
       error_str = [
-        'Catalog has broken references: exec[subscribe caller 1](/modules/test/manifests/subscribe_callers.pp:2)' \
+        'Catalog has broken references: exec[subscribe caller 1](modules/test/manifests/subscribe_callers.pp:2)' \
           ' -> subscribe[Exec[subscribe target]]',
-        'exec[subscribe caller 2](/modules/test/manifests/subscribe_callers.pp:7) -> subscribe[Exec[subscribe target]]',
-        'exec[subscribe caller 2](/modules/test/manifests/subscribe_callers.pp:7) -> subscribe[Exec[subscribe target 2]]',
-        'exec[subscribe caller 3](/modules/test/manifests/subscribe_callers.pp:15) -> subscribe[Exec[subscribe target]]'
+        'exec[subscribe caller 2](modules/test/manifests/subscribe_callers.pp:7) -> subscribe[Exec[subscribe target]]',
+        'exec[subscribe caller 2](modules/test/manifests/subscribe_callers.pp:7) -> subscribe[Exec[subscribe target 2]]',
+        'exec[subscribe caller 3](modules/test/manifests/subscribe_callers.pp:15) -> subscribe[Exec[subscribe target]]'
       ].join('; ')
       expect { catalog.validate_references }.to raise_error(OctocatalogDiff::Errors::ReferenceValidationError, error_str)
     end
@@ -518,7 +518,7 @@
           target_value: 'bar'
         }
         result = @test_obj.send(:format_missing_references, [obj])
-        expect(result).to eq('baz[buzz](/modules/foo/manifests/bar.pp:23) -> foo[bar]')
+        expect(result).to eq('baz[buzz](modules/foo/manifests/bar.pp:23) -> foo[bar]')
       end
     end