Eliminate ReferenceValidationError to mimic Puppet 5 behavior

Author: kpaulisse

lib/octocatalog-diff/catalog.rb

@@ -33,15 +33,18 @@ class Catalog
     def initialize(options = {})
       @options = options
 
+      # The compilation directory can be overridden, e.g. when testing
+      @override_compilation_dir = options[:compilation_dir]
+
       # Call appropriate backend for catalog generation
       @catalog_obj = backend(options)
 
       # The catalog is not built yet, except if the backend has no build method
       @built = false
       build unless @catalog_obj.respond_to?(:build)
 
-      # The compilation directory can be overridden, e.g. when testing
-      @override_compilation_dir = nil
+      # Keep track of whether references have been validated yet
+      @references_validated = false
     end
 
     # Build catalog - this method needs to be called to build the catalog. It is separate due to
@@ -69,6 +72,10 @@ def build(logger = Logger.new(StringIO.new))
 
       # Perform post-generation processing of the catalog
       return unless valid?
+
+      validate_references
+      return unless valid?
+
       unless @catalog_obj.respond_to?(:convert_file_resources) && @catalog_obj.convert_file_resources == false
         if @options.fetch(:compare_file_text, false)
           OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(self, environment)
@@ -182,6 +189,10 @@ def valid?
     # Raise a OctocatalogDiff::Errors::ReferenceValidationError for any found to be missing.
     # Uses @options[:validate_references] to influence which references are checked.
     def validate_references
+      # If we've already done the validation, don't do it again
+      return if @references_validated
+      @references_validated = true
+
       # Skip out early if no reference validation has been requested.
       unless @options[:validate_references].is_a?(Array) && @options[:validate_references].any?
         return
@@ -208,7 +219,7 @@ def validate_references
       # At this point there is at least one broken/missing reference. Format an error message and raise.
       errors = format_missing_references(missing)
       plural = errors =~ /;/ ? 's' : ''
-      raise OctocatalogDiff::Errors::ReferenceValidationError, "Catalog has broken reference#{plural}: #{errors}"
+      self.error_message = "Catalog has broken reference#{plural}: #{errors}"
     end
 
     private

spec/octocatalog-diff/tests/catalog_spec.rb

@@ -468,31 +468,33 @@
         compare_file_text: false,
         validate_references: %w(before notify require subscribe),
         node: 'my.rspec.node',
-        json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/reference-validation-broken.json'))
+        json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/reference-validation-broken.json')),
+        compilation_dir: '/var/folders/dw/5ftmkqk972j_kw2fdjyzdqdw0000gn/T/d20161223-46780-x10xaf/environments/production'
       }
+      allow_any_instance_of(OctocatalogDiff::Catalog).to receive(:puppet_version).and_return('5.0.0')
       catalog = OctocatalogDiff::Catalog.new(opts)
-      allow(catalog).to receive(:puppet_version).and_return('5.0.0')
-      expect { catalog.validate_references }.not_to raise_error
+      expect(catalog.valid?).to eq(true)
     end
 
-    it 'should raise error if reference validation is requested' do
+    it 'should indicate the invalid catalog if reference validation is requested' do
       opts = {
         compare_file_text: false,
         validate_references: %w(before notify require subscribe),
         node: 'my.rspec.node',
-        json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/reference-validation-broken.json'))
+        json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/reference-validation-broken.json')),
+        compilation_dir: '/var/folders/dw/5ftmkqk972j_kw2fdjyzdqdw0000gn/T/d20161223-46780-x10xaf/environments/production'
       }
+      allow_any_instance_of(OctocatalogDiff::Catalog).to receive(:puppet_version).and_return('4.10.0')
       catalog = OctocatalogDiff::Catalog.new(opts)
-      allow(catalog).to receive(:puppet_version).and_return('4.10.0')
-      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)' \
           ' -> 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)
+      expect(catalog.valid?).to eq(false)
+      expect(catalog.error_message).to eq(error_str)
     end
   end