Merge pull request #5653 from thallgren/issue/pup-7130/unify-ruby-int…

…eger-types

(PUP-7130) Unify ruby Integer types, ENV['HOME'] handling and HTTP time

lib/puppet/file_serving/http_metadata.rb

@@ -23,7 +23,7 @@ def initialize(http_response, path = '/dev/null')
 
     if last_modified = http_response['last-modified']
       mtime = DateTime.httpdate(last_modified).to_time
-      @checksums[:mtime] = "{mtime}#{mtime}"
+      @checksums[:mtime] = "{mtime}#{mtime.utc}"
     end
 
     @ftype = 'file'

lib/puppet/indirector/indirection.rb

@@ -60,7 +60,7 @@ def delete
 
   # Set the time-to-live for instances created through this indirection.
   def ttl=(value)
-    raise ArgumentError, "Indirection TTL must be an integer" unless value.is_a?(Fixnum)
+    raise ArgumentError, "Indirection TTL must be an integer" unless value.is_a?(Integer)
     @ttl = value
   end
 

lib/puppet/indirector/node/ldap.rb

@@ -165,7 +165,7 @@ def convert_parameters(parameters)
   # Convert any values if necessary.
   def convert(value)
     case value
-    when Integer, Fixnum, Bignum; value
+    when Integer; value
     when "true"; true
     when "false"; false
     else

lib/puppet/indirector/request.rb

@@ -133,7 +133,7 @@ def expand_primitive_types_into_parameters(data)
       case value
       when nil
         params
-      when true, false, String, Symbol, Fixnum, Bignum, Float
+      when true, false, String, Symbol, Integer, Float
         params << [key, value]
       else
         raise ArgumentError, "HTTP REST queries cannot handle values of type '#{value.class}'"

lib/puppet/network/http/connection.rb

@@ -30,14 +30,14 @@ class Connection
 
     # Creates a new HTTP client connection to `host`:`port`.
     # @param host [String] the host to which this client will connect to
-    # @param port [Fixnum] the port to which this client will connect to
+    # @param port [Integer] the port to which this client will connect to
     # @param options [Hash] options influencing the properties of the created
     #   connection,
     # @option options [Boolean] :use_ssl true to connect with SSL, false
     #   otherwise, defaults to true
     # @option options [#setup_connection] :verify An object that will configure
     #   any verification to do on the connection
-    # @option options [Fixnum] :redirect_limit the number of allowed
+    # @option options [Integer] :redirect_limit the number of allowed
     #   redirections, defaults to 10 passing any other option in the options
     #   hash results in a Puppet::Error exception
     #

lib/puppet/parser/scope.rb

@@ -864,7 +864,7 @@ def unset_ephemeral_var(level=:all)
 
   # Pop ephemeral scopes up to level and return them
   #
-  # @param level [Fixnum] a positive integer
+  # @param level [Integer] a positive integer
   # @return [Array] the removed ephemeral scopes
   # @api private
   def pop_ephemerals(level)

lib/puppet/pops/evaluator/puppet_proc.rb

@@ -56,7 +56,7 @@ def parameters
     end
   end
 
-  # @return [Fixnum] the arity of the block
+  # @return [Integer] the arity of the block
   # @overrides Block.arity
   # @api public
   def arity

lib/puppet/provider/augeas/augeas.rb

@@ -203,7 +203,7 @@ def close_augeas
 
   def is_numeric?(s)
     case s
-    when Fixnum
+    when Integer
       true
     when String
       s.match(/\A[+-]?\d+?(\.\d+)?\Z/n) == nil ? false : true

lib/puppet/provider/user/aix.rb

@@ -167,7 +167,7 @@ def groupid_by_name(groupname)
 
   # Check that a group exists and is valid
   def verify_group(value)
-    if value.is_a? Integer or value.is_a? Fixnum
+    if value.is_a? Integer
       groupname = groupname_by_id(value)
       raise ArgumentError, "AIX group must be a valid existing group" unless groupname
     else

lib/puppet/provider/user/ldap.rb

@@ -46,7 +46,7 @@
 
   # Convert our gid to a group name, if necessary.
   def gid=(value)
-    value = group2id(value) unless [Fixnum, Bignum].include?(value.class)
+    value = group2id(value) unless value.is_a?(Integer)
 
     @property_hash[:gid] = value
   end

lib/puppet/ssl/certificate_factory.rb

@@ -30,7 +30,7 @@ def self.build(cert_type, csr, issuer, serial, ttl = nil)
     respond_to?(build_extensions) or
       raise ArgumentError, "#{cert_type.to_s} is an invalid certificate type!"
 
-    raise ArgumentError, "Certificate TTL must be an integer" unless ttl.nil? || ttl.is_a?(Fixnum)
+    raise ArgumentError, "Certificate TTL must be an integer" unless ttl.nil? || ttl.is_a?(Integer)
 
     # set up the certificate, and start building the content.
     cert = OpenSSL::X509::Certificate.new

lib/puppet/type/file.rb

@@ -195,7 +195,7 @@ def self.title_patterns
     munge do |value|
       newval = super(value)
       case newval
-      when Integer, Fixnum, Bignum; value
+      when Integer; value
       when /^\d+$/; Integer(value)
       else
         self.fail "Invalid recurselimit value #{value.inspect}"

lib/puppet/type/tidy.rb

@@ -42,7 +42,7 @@
       case newval
       when :true, :inf; true
       when :false; false
-      when Integer, Fixnum, Bignum; value
+      when Integer; value
       when /^\d+$/; Integer(value)
       else
         raise ArgumentError, "Invalid recurse value #{value.inspect}"
@@ -243,7 +243,7 @@ def generate
     return [] unless stat(self[:path])
 
     case self[:recurse]
-    when Integer, Fixnum, Bignum, /^\d+$/
+    when Integer, /^\d+$/
       parameter = { :recurse => true, :recurselimit => self[:recurse] }
     when true, :true, :inf
       parameter = { :recurse => true }

lib/puppet/vendor/rgen/lib/rgen/metamodel_builder/data_types.rb

@@ -67,7 +67,7 @@ def to_s # :nodoc:
 
 	# Long represents a 64-bit Integer
   # This constant is merely a marker for keeping this information in the Ruby version of the metamodel,
-  # values of this type will always be instances of Integer or Bignum;
+  # values of this type will always be instances of Integer;
   # Setting it to a string value ensures that it responds to "to_s" which is used in the metamodel generator
 	Long = "Long"
 end

spec/unit/file_bucket/file_spec.rb

@@ -28,7 +28,7 @@
   end
 
   it "should require contents to be a string" do
-    expect { Puppet::FileBucket::File.new(5) }.to raise_error(ArgumentError, /contents must be a String or Pathname, got a Fixnum$/)
+    expect { Puppet::FileBucket::File.new(5) }.to raise_error(ArgumentError, /contents must be a String or Pathname, got a (?:Fixnum|Integer)$/)
   end
 
   it "should complain about options other than :bucket_path" do

spec/unit/file_serving/http_metadata_spec.rb

@@ -61,7 +61,7 @@
         metadata = described_class.new(http_response)
         metadata.collect
         expect( metadata.checksum_type ).to eq :mtime
-        expect( metadata.checksum ).to eq "{mtime}#{time.to_time}"
+        expect( metadata.checksum ).to eq "{mtime}#{time.to_time.utc}"
       end
     end
 

spec/unit/functions4_spec.rb

@@ -200,7 +200,7 @@ def test(x)
     it 'a function can use inexact argument mapping' do
       f = create_function_with_inexact_dispatch
       func = f.new(:closure_scope, :loader)
-      expect(func.call({}, 3,4,5)).to eql([Fixnum, Fixnum, Fixnum])
+      expect(func.call({}, 3.0,4.0,5.0)).to eql([Float, Float, Float])
       expect(func.call({}, 'Apple', 'Banana')).to eql([String, String])
     end
 

spec/unit/parser/scope_spec.rb

@@ -418,8 +418,8 @@ def create_class_scope(name)
       expect(Puppet::Parser::Scope.number?([2])).to be_nil
     end
 
-    it "should return a Fixnum for a Fixnum" do
-      expect(Puppet::Parser::Scope.number?(2)).to be_an_instance_of(Fixnum)
+    it "should return a Integer for an Integer" do
+      expect(Puppet::Parser::Scope.number?(2)).to be_a(Integer)
     end
 
     it "should return a Float for a Float" do

spec/unit/pops/evaluator/evaluating_parser_spec.rb

@@ -743,19 +743,22 @@
       end
     end
 
+    # Integer for >= 2.4.0, otherwise Fixnum
+    int_class_name = 0.class.name
+
     # Errors when wrong number/type of keys are used
     {
-      "Array[0]"                    => 'Array-Type[] arguments must be types. Got Fixnum',
-      "Hash[0]"                     => 'Hash-Type[] arguments must be types. Got Fixnum',
-      "Hash[Integer, 0]"            => 'Hash-Type[] arguments must be types. Got Fixnum',
+      "Array[0]"                    => "Array-Type[] arguments must be types. Got #{int_class_name}",
+      "Hash[0]"                     => "Hash-Type[] arguments must be types. Got #{int_class_name}",
+      "Hash[Integer, 0]"            => "Hash-Type[] arguments must be types. Got #{int_class_name}",
       "Array[Integer,1,2,3]"        => 'Array-Type[] accepts 1 to 3 arguments. Got 4',
       "Array[Integer,String]"       => "A Type's size constraint arguments must be a single Integer type, or 1-2 integers (or default). Got a String-Type",
       "Hash[Integer,String, 1,2,3]" => 'Hash-Type[] accepts 2 to 4 arguments. Got 5',
       "'abc'[x]"                    => "A substring operation does not accept a String as a character index. Expected an Integer",
       "'abc'[1.0]"                  => "A substring operation does not accept a Float as a character index. Expected an Integer",
       "'abc'[1, x]"                 => "A substring operation does not accept a String as a character index. Expected an Integer",
       "'abc'[1,2,3]"                => "String supports [] with one or two arguments. Got 3",
-      "NotUndef[0]"                 => 'NotUndef-Type[] argument must be a Type or a String. Got Fixnum',
+      "NotUndef[0]"                 => "NotUndef-Type[] argument must be a Type or a String. Got #{int_class_name}",
       "NotUndef[a,b]"               => 'NotUndef-Type[] accepts 0 to 1 arguments. Got 2',
       "Resource[0]"                 => 'First argument to Resource[] must be a resource type or a String. Got Integer',
       "Resource[a, 0]"              => 'Error creating type specialization of a Resource-Type, Cannot use Integer where a resource title String is expected',

spec/unit/pops/types/type_calculator_spec.rb

@@ -1629,12 +1629,12 @@ class Bar < Foo
       types_to_test.each {|t| expect(calculator.instance?(t::DEFAULT, :default)).to eq(false) }
     end
 
-    it 'should consider fixnum instanceof PIntegerType' do
+    it 'should consider integer instanceof PIntegerType' do
       expect(calculator.instance?(PIntegerType::DEFAULT, 1)).to eq(true)
     end
 
-    it 'should consider fixnum instanceof Fixnum' do
-      expect(calculator.instance?(Fixnum, 1)).to eq(true)
+    it 'should consider integer instanceof Integer' do
+      expect(calculator.instance?(Integer, 1)).to eq(true)
     end
 
     it 'should consider integer in range' do
@@ -1885,10 +1885,16 @@ def foo(a)
   end
 
   context 'when converting a ruby class' do
-    it 'should yield \'PIntegerType\' for Integer, Fixnum, and Bignum' do
-      [Integer,Fixnum,Bignum].each do |c|
-        expect(calculator.type(c).class).to eq(PIntegerType)
-      end
+    it 'should yield \'PIntegerType\' for Fixnum' do
+      expect(calculator.type(Fixnum).class).to eq(PIntegerType)
+    end
+
+    it 'should yield \'PIntegerType\' for Bignum' do
+      expect(calculator.type(Bignum).class).to eq(PIntegerType)
+    end
+
+    it 'should yield \'PIntegerType\' for Integer' do
+      expect(calculator.type(Integer).class).to eq(PIntegerType)
     end
 
     it 'should yield \'PFloatType\' for Float' do

spec/unit/pops/types/type_factory_spec.rb

@@ -176,11 +176,11 @@ module Types
       expect(ht.value_type.class).to eq(PDataType)
     end
 
-    it 'ruby(1) returns PRuntimeType[ruby, \'Fixnum\']' do
+    it 'ruby(1) returns PRuntimeType[ruby, \'Integer\']' do
       ht = TypeFactory.ruby(1)
       expect(ht.class()).to eq(PRuntimeType)
       expect(ht.runtime).to eq(:ruby)
-      expect(ht.runtime_type_name).to eq('Fixnum')
+      expect(ht.runtime_type_name).to eq(1.class.name)
     end
 
     it 'a size constrained collection can be created from array' do

spec/unit/pops/visitor_spec.rb

@@ -40,7 +40,7 @@ class Duck
 
     it "should select method for superclass" do
       duck_processor = DuckProcessor.new
-      expect(duck_processor.hi(42)).to eq("Howdy Fixnum")
+      expect(duck_processor.hi(42)).to match(/Howdy (?:Fixnum|Integer)/)
     end
 
     it "should select method for superclass" do

spec/unit/provider/augeas/augeas_spec.rb

@@ -367,7 +367,7 @@
       it "should return false for nil" do
         expect(@provider.is_numeric?(nil)).to eq(false)
       end
-      it "should return true for Fixnums" do
+      it "should return true for Integers" do
         expect(@provider.is_numeric?(9)).to eq(true)
       end
       it "should return true for numbers in Strings" do

spec/unit/provider/user/directoryservice_spec.rb

@@ -157,7 +157,7 @@ module Puppet::Util::Plist
     "9377c46908a1c8ac2c3e45c0d44da8ad0fcd85ec5c14d9a59ffc40c9da31f0ec"
   end
 
-  # The below value is a Fixnum iterations value used in the PBKDF2
+  # The below value is an Integer iterations value used in the PBKDF2
   # key stretching algorithm
   let(:pbkdf2_iterations_value) do
     24752
@@ -394,12 +394,12 @@ module Puppet::Util::Plist
       provider.class.prefetch({})
     end
 
-    it 'should return :uid values as a Fixnum' do
-      expect(provider.class.generate_attribute_hash(user_plist_hash)[:uid]).to be_a_kind_of Fixnum
+    it 'should return :uid values as an Integer' do
+      expect(provider.class.generate_attribute_hash(user_plist_hash)[:uid]).to be_a Integer
     end
 
-    it 'should return :gid values as a Fixnum' do
-      expect(provider.class.generate_attribute_hash(user_plist_hash)[:gid]).to be_a_kind_of Fixnum
+    it 'should return :gid values as an Integer' do
+      expect(provider.class.generate_attribute_hash(user_plist_hash)[:gid]).to be_a Integer
     end
 
     it 'should return a hash of resource attributes' do
@@ -799,8 +799,8 @@ module Puppet::Util::Plist
     it "should accept a hash containing a PBKDF2 password hash, salt, and iterations value and return the correct iterations value" do
         expect(provider.class.get_salted_sha512_pbkdf2('iterations', pbkdf2_embedded_bplist_hash)).to eq(pbkdf2_iterations_value)
     end
-    it "should return a Fixnum value when looking up the PBKDF2 iterations value" do
-        expect(provider.class.get_salted_sha512_pbkdf2('iterations', pbkdf2_embedded_bplist_hash)).to be_a_kind_of(Fixnum)
+    it "should return an Integer value when looking up the PBKDF2 iterations value" do
+        expect(provider.class.get_salted_sha512_pbkdf2('iterations', pbkdf2_embedded_bplist_hash)).to be_a(Integer)
     end
     it "should raise an error if a field other than 'entropy', 'salt', or 'iterations' is passed" do
       expect { provider.class.get_salted_sha512_pbkdf2('othervalue', pbkdf2_embedded_bplist_hash) }.to raise_error(Puppet::Error, /Puppet has tried to read an incorrect value from the SALTED-SHA512-PBKDF2 hash. Acceptable fields are 'salt', 'entropy', or 'iterations'/)

spec/unit/type/file/mode_spec.rb

@@ -13,7 +13,7 @@
     it "should reject non-string values" do
       expect {
         mode.value = 0755
-      }.to raise_error(Puppet::Error, /The file mode specification must be a string, not 'Fixnum'/)
+      }.to raise_error(Puppet::Error, /The file mode specification must be a string, not '(?:Fixnum|Integer)'/)
     end
 
     it "should accept values specified as octal numbers in strings" do

spec/unit/util/run_mode_spec.rb

@@ -2,6 +2,13 @@
 require 'spec_helper'
 
 describe Puppet::Util::RunMode do
+
+  # Discriminator for tests that attempts to unset HOME since that, for reasons currently unknown,
+  # doesn't work in Ruby >= 2.4.0
+  def self.gte_ruby_2_4
+    @gte_ruby_2_4 ||= SemanticPuppet::Version.parse(RUBY_VERSION) >= SemanticPuppet::Version.parse('2.4.0')
+  end
+
   before do
     @run_mode = Puppet::Util::RunMode.new('fake')
   end
@@ -29,7 +36,7 @@
         end
       end
 
-      it "fails when asking for the conf_dir as non-root and there is no $HOME" do
+      it "fails when asking for the conf_dir as non-root and there is no $HOME", :unless => gte_ruby_2_4 || Puppet.features.microsoft_windows? do
         as_non_root do
           without_home do
             expect { @run_mode.conf_dir }.to raise_error ArgumentError, /couldn't find HOME/
@@ -57,7 +64,7 @@
         end
       end
 
-      it "fails when asking for the code_dir as non-root and there is no $HOME" do
+      it "fails when asking for the code_dir as non-root and there is no $HOME", :unless => gte_ruby_2_4 || Puppet.features.microsoft_windows? do
         as_non_root do
           without_home do
             expect { @run_mode.code_dir }.to raise_error ArgumentError, /couldn't find HOME/
@@ -75,7 +82,7 @@
         as_non_root { expect(@run_mode.var_dir).to eq(File.expand_path('~/.puppetlabs/opt/puppet/cache')) }
       end
 
-      it "fails when asking for the var_dir as non-root and there is no $HOME" do
+      it "fails when asking for the var_dir as non-root and there is no $HOME", :unless => gte_ruby_2_4 || Puppet.features.microsoft_windows? do
         as_non_root do
           without_home do
             expect { @run_mode.var_dir }.to raise_error ArgumentError, /couldn't find HOME/
@@ -96,7 +103,7 @@
           as_non_root { expect(@run_mode.log_dir).to eq(File.expand_path('~/.puppetlabs/var/log')) }
         end
 
-        it "fails when asking for the log_dir and there is no $HOME" do
+        it "fails when asking for the log_dir and there is no $HOME", :unless => gte_ruby_2_4 || Puppet.features.microsoft_windows? do
           as_non_root do
             without_home do
               expect { @run_mode.log_dir }.to raise_error ArgumentError, /couldn't find HOME/
@@ -118,7 +125,7 @@
           as_non_root { expect(@run_mode.run_dir).to eq(File.expand_path('~/.puppetlabs/var/run')) }
         end
 
-        it "fails when asking for the run_dir and there is no $HOME" do
+        it "fails when asking for the run_dir and there is no $HOME", :unless => gte_ruby_2_4 || Puppet.features.microsoft_windows? do
           as_non_root do
             without_home do
               expect { @run_mode.run_dir }.to raise_error ArgumentError, /couldn't find HOME/