(CONT-237) Rubocop updates

lib/puppetlabs_spec_helper/module_spec_helper.rb

@@ -53,9 +53,9 @@ def verify_contents(subject, title, expected_lines)
 end
 
 # Add all spec lib dirs to LOAD_PATH
-components = module_path.split(File::PATH_SEPARATOR).collect do |dir|
+components = module_path.split(File::PATH_SEPARATOR).map do |dir|
   next unless Dir.exist? dir
-  Dir.entries(dir).reject { |f| f =~ %r{^\.} }.collect { |f| File.join(dir, f, 'spec', 'lib') }
+  Dir.entries(dir).reject { |f| f =~ %r{^\.} }.map { |f| File.join(dir, f, 'spec', 'lib') }
 end
 components.flatten.each do |d|
   $LOAD_PATH << d if FileTest.directory?(d) && !$LOAD_PATH.include?(d)

lib/puppetlabs_spec_helper/puppet_spec_helper.rb

@@ -48,62 +48,63 @@
 begin
   require 'puppet/test/test_helper'
 rescue LoadError
+  # Continue gracefully
 end
 
 # This is just a utility class to allow us to isolate the various version-specific
 # branches of initialization logic into methods without polluting the global namespace.#
-module Puppet
-  class PuppetSpecInitializer
-    # This method is for initializing puppet state for testing for older versions
-    # of puppet that do not support the new TestHelper API.  As you can see,
-    # this involves explicitly modifying global variables, directly manipulating
-    # Puppet's Settings singleton object, and other fun implementation details
-    # that code external to puppet should really never know about.
-    def self.initialize_via_fallback_compatibility(config)
-      warn('Warning: you appear to be using an older version of puppet; spec_helper will use fallback compatibility mode.')
-      config.before :all do
-        # nothing to do for now
-      end
+class Puppet::PuppetSpecInitializer
+  # This method is for initializing puppet state for testing for older versions
+  # of puppet that do not support the new TestHelper API.  As you can see,
+  # this involves explicitly modifying global variables, directly manipulating
+  # Puppet's Settings singleton object, and other fun implementation details
+  # that code external to puppet should really never know about.
+  def self.initialize_via_fallback_compatibility(config)
+    warn('Warning: you appear to be using an older version of puppet; spec_helper will use fallback compatibility mode.')
+    config.before :all do
+      # nothing to do for now
+    end
 
-      config.after :all do
-        # nothing to do for now
-      end
+    config.after :all do
+      # nothing to do for now
+    end
 
-      config.before :each do
-        # these globals are set by Application
-        $puppet_application_mode = nil
-        $puppet_application_name = nil
+    config.before :each do
+      # these globals are set by Application
+      $puppet_application_mode = nil # rubocop:disable Style/GlobalVars
+      $puppet_application_name = nil # rubocop:disable Style/GlobalVars
 
-        # REVISIT: I think this conceals other bad tests, but I don't have time to
-        # fully diagnose those right now.  When you read this, please come tell me
-        # I suck for letting this float. --daniel 2011-04-21
-        Signal.stubs(:trap)
+      # REVISIT: I think this conceals other bad tests, but I don't have time to
+      # fully diagnose those right now.  When you read this, please come tell me
+      # I suck for letting this float. --daniel 2011-04-21
+      Signal.stubs(:trap)
 
-        # Set the confdir and vardir to gibberish so that tests
-        # have to be correctly mocked.
-        Puppet[:confdir] = '/dev/null'
-        Puppet[:vardir] = '/dev/null'
+      # Set the confdir and vardir to gibberish so that tests
+      # have to be correctly mocked.
+      Puppet[:confdir] = '/dev/null'
+      Puppet[:vardir] = '/dev/null'
 
-        # Avoid opening ports to the outside world
-        Puppet.settings[:bindaddress] = '127.0.0.1'
-      end
+      # Avoid opening ports to the outside world
+      Puppet.settings[:bindaddress] = '127.0.0.1'
+    end
 
-      config.after :each do
-        Puppet.settings.clear
+    config.after :each do
+      Puppet.settings.clear
 
-        Puppet::Node::Environment.clear
-        Puppet::Util::Storage.clear
-        Puppet::Util::ExecutionStub.reset if Puppet::Util.constants.include? 'ExecutionStub'
+      Puppet::Node::Environment.clear
+      Puppet::Util::Storage.clear
+      Puppet::Util::ExecutionStub.reset if Puppet::Util.constants.include? 'ExecutionStub'
 
-        PuppetlabsSpec::Files.cleanup
-      end
+      PuppetlabsSpec::Files.cleanup
     end
   end
 end
 
 # JJM Hack to make the stdlib tests run in Puppet 2.6 (See puppet commit cf183534)
 unless Puppet.constants.include? 'Test'
   module Puppet::Test
+    # This is a stub class to allow the stdlib tests to run in Puppet 2.6
+    # This class will be removed in another commit.
     class LogCollector
       def initialize(logs)
         @logs = logs

lib/puppetlabs_spec_helper/puppetlabs_spec/files.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# rubocop:disable Style/GlobalVars # TODO: investigate the use of instance variables
+
 require 'fileutils'
 require 'tempfile'
 require 'pathname'
@@ -20,7 +22,7 @@ def self.in_tmp(path)
 
   def self.cleanup
     $global_tempfiles ||= []
-    while path = $global_tempfiles.pop
+    while (path = $global_tempfiles.pop)
       raise "Not deleting tmpfile #{path} outside regular tmpdir" unless in_tmp(path)
 
       begin

lib/puppetlabs_spec_helper/puppetlabs_spec/fixtures.rb

@@ -14,8 +14,8 @@ def fixtures(*rest)
   # <project>/spec/fixture/unit/facter/foo
   def my_fixture_dir
     callers = caller
-    while line = callers.shift
-      next unless found = line.match(%r{/spec/(.*)_spec\.rb:})
+    while (line = callers.shift)
+      next unless (found = line.match(%r{/spec/(.*)_spec\.rb:}))
 
       return fixtures(found[1])
     end
@@ -47,7 +47,7 @@ def my_fixtures(glob = '*', flags = 0, &block)
       raise "fixture '#{glob}' for #{my_fixture_dir} had no files!"
     end
 
-    block_given? && files.each(&block)
+    block && files.each(&block)
     files
   end
 end

lib/puppetlabs_spec_helper/puppetlabs_spec/matchers.rb

@@ -5,20 +5,15 @@
 
 ########################################################################
 # Backward compatibility for Jenkins outdated environment.
-module RSpec
-  module Matchers
-    module BlockAliases
-      if method_defined?(:should) && !method_defined?(:to)
-        alias to should
-      end
-      if method_defined? :should_not
-        alias to_not should_not unless method_defined? :to_not
-        alias not_to should_not unless method_defined? :not_to
-      end
-    end
+module RSpec::Matchers::BlockAliases
+  if method_defined?(:should) && !method_defined?(:to)
+    alias to should
+  end
+  if method_defined? :should_not
+    alias to_not should_not unless method_defined? :to_not
+    alias not_to should_not unless method_defined? :not_to
   end
 end
-
 ########################################################################
 # Custom matchers...
 RSpec::Matchers.define :have_matching_element do |expected|

lib/puppetlabs_spec_helper/puppetlabs_spec/puppet_internals.rb

@@ -4,74 +4,73 @@
 # 'puppetlabs_spec_helper/puppet_spec_helper' library
 require 'puppetlabs_spec_helper/puppet_spec_helper'
 
-module PuppetlabsSpec
-  module PuppetInternals
-    # parser_scope is intended to return a Puppet::Parser::Scope
-    # instance suitable for placing in a test harness with the intent of
-    # testing parser functions from modules.
-    def scope(parts = {})
-      RSpec.deprecate('scope', replacement: 'rspec-puppet 2.2.0 provides a scope property')
+# PuppetInternals provides a set of methods that interface
+# with internal puppet implementations.
+module PuppetlabsSpec::PuppetInternals
+  # parser_scope is intended to return a Puppet::Parser::Scope
+  # instance suitable for placing in a test harness with the intent of
+  # testing parser functions from modules.
+  def scope(parts = {})
+    RSpec.deprecate('scope', replacement: 'rspec-puppet 2.2.0 provides a scope property')
 
-      if %r{^2\.[67]}.match?(Puppet.version)
-        # loadall should only be necessary prior to 3.x
-        # Please note, loadall needs to happen first when creating a scope, otherwise
-        # you might receive undefined method `function_*' errors
-        Puppet::Parser::Functions.autoloader.loadall
-      end
+    if %r{^2\.[67]}.match?(Puppet.version)
+      # loadall should only be necessary prior to 3.x
+      # Please note, loadall needs to happen first when creating a scope, otherwise
+      # you might receive undefined method `function_*' errors
+      Puppet::Parser::Functions.autoloader.loadall
+    end
 
-      scope_compiler = parts[:compiler] || compiler
-      scope_parent = parts[:parent] || scope_compiler.topscope
-      scope_resource = parts[:resource] || resource(type: :node, title: scope_compiler.node.name)
+    scope_compiler = parts[:compiler] || compiler
+    scope_parent = parts[:parent] || scope_compiler.topscope
 
-      scope = if %r{^2\.[67]}.match?(Puppet.version)
-                Puppet::Parser::Scope.new(compiler: scope_compiler)
-              else
-                Puppet::Parser::Scope.new(scope_compiler)
-              end
+    scope = if %r{^2\.[67]}.match?(Puppet.version)
+              Puppet::Parser::Scope.new(compiler: scope_compiler)
+            else
+              Puppet::Parser::Scope.new(scope_compiler)
+            end
 
-      scope.source = Puppet::Resource::Type.new(:node, 'foo')
-      scope.parent = scope_parent
-      scope
-    end
-    module_function :scope
+    scope.source = Puppet::Resource::Type.new(:node, 'foo')
+    scope.parent = scope_parent
+    scope
+  end
+  module_function :scope
 
-    def resource(parts = {})
-      resource_type = parts[:type] || :hostclass
-      resource_name = parts[:name] || 'testing'
-      Puppet::Resource::Type.new(resource_type, resource_name)
-    end
-    module_function :resource
+  def resource(parts = {})
+    resource_type = parts[:type] || :hostclass
+    resource_name = parts[:name] || 'testing'
+    Puppet::Resource::Type.new(resource_type, resource_name)
+  end
+  module_function :resource
 
-    def compiler(parts = {})
-      compiler_node = parts[:node] || node
-      Puppet::Parser::Compiler.new(compiler_node)
-    end
-    module_function :compiler
+  def compiler(parts = {})
+    compiler_node = parts[:node] || node
+    Puppet::Parser::Compiler.new(compiler_node)
+  end
+  module_function :compiler
 
-    def node(parts = {})
-      node_name = parts[:name] || 'testinghost'
-      options = parts[:options] || {}
-      node_environment = if Puppet.version.to_f >= 4.0
-                           Puppet::Node::Environment.create(parts[:environment] || 'test', [])
-                         else
-                           Puppet::Node::Environment.new(parts[:environment] || 'test')
-                         end
-      options[:environment] = node_environment
-      Puppet::Node.new(node_name, options)
-    end
-    module_function :node
+  def node(parts = {})
+    node_name = parts[:name] || 'testinghost'
+    options = parts[:options] || {}
+    node_environment = if Puppet.version.to_f >= 4.0
+                         Puppet::Node::Environment.create(parts[:environment] || 'test', [])
+                       else
+                         Puppet::Node::Environment.new(parts[:environment] || 'test')
+                       end
+    options[:environment] = node_environment
+    Puppet::Node.new(node_name, options)
+  end
+  module_function :node
 
-    # Return a method instance for a given function.  This is primarily useful
-    # for rspec-puppet
-    def function_method(name, parts = {})
-      scope = parts[:scope] || scope()
-      # Ensure the method instance is defined by side-effect of checking if it
-      # exists.  This is a hack, but at least it's a hidden hack and not an
-      # exposed hack.
-      return nil unless Puppet::Parser::Functions.function(name)
+  # Return a method instance for a given function.  This is primarily useful
+  # for rspec-puppet
+  def function_method(name, parts = {})
+    scope = parts[:scope] || scope()
+    # Ensure the method instance is defined by side-effect of checking if it
+    # exists.  This is a hack, but at least it's a hidden hack and not an
+    # exposed hack.
+    return nil unless Puppet::Parser::Functions.function(name)
 
-      scope.method("function_#{name}".intern)
-    end
-    module_function :function_method
+    scope.method("function_#{name}".to_sym)
   end
+  module_function :function_method
 end