Merge pull request #125 from github/kpaulisse-datatype-regression

Fix datatype regression for fixnums vs. integers

Author: kpaulisse

lib/octocatalog-diff/catalog-diff/differ.rb

@@ -8,6 +8,7 @@
 
 require_relative '../catalog'
 require_relative '../errors'
+require_relative '../util/util'
 require_relative 'filter'
 
 module OctocatalogDiff
@@ -530,7 +531,8 @@ def hashdiff_initial(catalog1_in, catalog2_in)
 
           # Added a new key that points to some kind of data structure that we know how
           # to handle.
-          if obj[1] =~ /^(.+)\f([^\f]+)$/ && [String, Integer, Float, TrueClass, FalseClass, Array, Hash].include?(obj[2].class)
+          classes = [String, Integer, Float, TrueClass, FalseClass, Array, Hash]
+          if obj[1] =~ /^(.+)\f([^\f]+)$/ && OctocatalogDiff::Util::Util.object_is_any_of?(obj[2], classes)
             hashdiff_add_remove.add(obj[1])
             next
           end

lib/octocatalog-diff/catalog-diff/display/text.rb

@@ -1,7 +1,8 @@
 # frozen_string_literal: true
 
 require_relative '../display'
-require_relative '../../util/colored.rb'
+require_relative '../../util/colored'
+require_relative '../../util/util'
 
 require 'diffy'
 require 'json'
@@ -447,26 +448,16 @@ def self.class_name_for_diffy(class_name)
           class_name
         end
 
-        # Utility Method!
-        # `is_a?(class)` only allows one method, but this uses an array
-        # @param object [?] Object to consider
-        # @param classes [Array] Classes to determine if object is a member of
-        # @return [Boolean] True if object is_a any of the classes, false otherwise
-        def self.object_is_any_of?(object, classes)
-          classes.each { |clazz| return true if object.is_a? clazz }
-          false
-        end
-
         # Utility Method!
         # Given an arbitrary object, convert it into a string for use by 'diffy'.
         # This basically exists so we can do something prettier than just calling .inspect or .to_s
         # on object types we anticipate seeing, while not failing entirely on other object types.
         # @param obj [?] Object to be stringified
         # @return [String] String representation of object for diffy
         def self.stringify_for_diffy(obj)
-          return JSON.pretty_generate(obj) if object_is_any_of?(obj, [Hash, Array])
+          return JSON.pretty_generate(obj) if OctocatalogDiff::Util::Util.object_is_any_of?(obj, [Hash, Array])
           return '""' if obj.is_a?(String) && obj == ''
-          return obj if object_is_any_of?(obj, [String, Fixnum, Integer, Float])
+          return obj if OctocatalogDiff::Util::Util.object_is_any_of?(obj, [String, Fixnum, Integer, Float])
           "#{class_name_for_diffy(obj.class)}: #{obj.inspect}"
         end
 

spec/octocatalog-diff/fixtures/catalogs/regression-fixnum-1.json

@@ -0,0 +1,26 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "name": "octoawesome-fe-00decaf.example.com",
+    "version": "",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "Special",
+        "title": "My::Silly::Class",
+        "tags": [
+          "class",
+          "my::silly::class",
+          "my",
+          "silly",
+          "class"
+        ],
+        "exported": false,
+        "parameters": {
+          "number_one": 7199,
+          "number_two": 2181
+        }
+      }
+    ]
+  }
+}

spec/octocatalog-diff/fixtures/catalogs/regression-fixnum-2.json

@@ -0,0 +1,28 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "name": "octoawesome-fe-00decaf.example.com",
+    "version": "",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "Special",
+        "title": "My::Silly::Class",
+        "tags": [
+          "class",
+          "my::silly::class",
+          "my",
+          "silly",
+          "class"
+        ],
+        "exported": false,
+        "parameters": {
+          "number_one": 7199,
+          "number_two": 2181,
+          "number_three": 1234,
+          "string": "boom"
+        }
+      }
+    ]
+  }
+}

spec/octocatalog-diff/integration/regressions_spec.rb

@@ -25,3 +25,18 @@
     expect(@result[:diffs].size).to eq(0), @result[:diffs].map(&:inspect).join("\n")
   end
 end
+
+describe 'resource with Fixnum parameters added' do
+  before(:all) do
+    @result = OctocatalogDiff::Integration.integration(
+      from_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/regression-fixnum-1.json'),
+      to_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/regression-fixnum-2.json')
+    )
+  end
+
+  it 'should run without an error' do
+    expect(@result[:exitcode]).not_to eq(-1), "Internal error: #{OctocatalogDiff::Integration.format_exception(@result)}"
+    expect(@result[:exitcode]).to eq(2), "Runtime error: #{@result[:logs]}"
+    expect(@result[:diffs].size).to eq(2), @result[:diffs].map(&:inspect).join("\n")
+  end
+end