Merge pull request #9129 from joshcooper/ruby_warning

(PUP-11974) Only warn when Ruby's verbose is enabled

lib/puppet/util/monkey_patches.rb

@@ -33,7 +33,7 @@ def daemonize
 unless Dir.singleton_methods.include?(:exists?)
   class Dir
     def self.exists?(file_name)
-      warn('exists? is a deprecated name, use exist? instead')
+      warn("Dir.exists?('#{file_name}') is deprecated, use Dir.exist? instead") if $VERBOSE
       Dir.exist?(file_name)
     end
   end
@@ -42,7 +42,7 @@ def self.exists?(file_name)
 unless File.singleton_methods.include?(:exists?)
   class File
     def self.exists?(file_name)
-      warn('exists? is a deprecated name, use exist? instead')
+      warn("File.exists?('#{file_name}') is deprecated, use File.exist? instead") if $VERBOSE
       File.exist?(file_name)
     end
   end

spec/lib/puppet_spec/verbose.rb

@@ -1,9 +1,20 @@
-# Support code for running stuff with warnings disabled.
+# Support code for running stuff with warnings disabled or enabled
 module Kernel
   def with_verbose_disabled
     verbose, $VERBOSE = $VERBOSE, nil
-    result = yield
-    $VERBOSE = verbose
-    return result
+    begin
+      yield
+    ensure
+      $VERBOSE = verbose
+    end
+  end
+
+  def with_verbose_enabled
+    verbose, $VERBOSE = $VERBOSE, true
+    begin
+      yield
+    ensure
+      $VERBOSE = verbose
+    end
   end
 end

spec/unit/agent_spec.rb

@@ -15,19 +15,12 @@ def stop
   end
 end
 
-def without_warnings
-  flag = $VERBOSE
-  $VERBOSE = nil
-  yield
-  $VERBOSE = flag
-end
-
 describe Puppet::Agent do
   before do
     @agent = Puppet::Agent.new(AgentTestClient, false)
 
-    # make Puppet::Application safe for stubbing; restore in an :after block; silence warnings for this.
-    without_warnings { Puppet::Application = Class.new(Puppet::Application) }
+    # make Puppet::Application safe for stubbing
+    stub_const('Puppet::Application', Class.new(Puppet::Application))
     allow(Puppet::Application).to receive(:clear?).and_return(true)
     Puppet::Application.class_eval do
       class << self
@@ -42,11 +35,6 @@ def controlled_run(&block)
     allow(Puppet::SSL::StateMachine).to receive(:new).and_return(machine)
   end
 
-  after do
-    # restore Puppet::Application from stub-safe subclass, and silence warnings
-    without_warnings { Puppet::Application = Puppet::Application.superclass }
-  end
-
   it "should set its client class at initialization" do
     expect(Puppet::Agent.new("foo", false).client_class).to eq("foo")
   end

spec/unit/daemon_spec.rb

@@ -3,13 +3,6 @@
 require 'puppet/agent'
 require 'puppet/configurer'
 
-def without_warnings
-  flag = $VERBOSE
-  $VERBOSE = nil
-  yield
-  $VERBOSE = flag
-end
-
 describe Puppet::Daemon, :unless => Puppet::Util::Platform.windows? do
   include PuppetSpec::Files
 
@@ -91,14 +84,8 @@ def run_loop(jobs)
   describe "when stopping" do
     before do
       allow(Puppet::Util::Log).to receive(:close_all)
-      # to make the global safe to mock, set it to a subclass of itself,
-      # then restore it in an after pass
-      without_warnings { Puppet::Application = Class.new(Puppet::Application) }
-    end
-
-    after do
-      # restore from the superclass so we lose the stub garbage
-      without_warnings { Puppet::Application = Puppet::Application.superclass }
+      # to make the global safe to mock, set it to a subclass of itself
+      stub_const('Puppet::Application', Class.new(Puppet::Application))
     end
 
     it 'should request a stop from Puppet::Application' do
@@ -143,11 +130,7 @@ def run_loop(jobs)
 
   describe "when restarting" do
     before do
-      without_warnings { Puppet::Application = Class.new(Puppet::Application) }
-    end
-
-    after do
-      without_warnings { Puppet::Application = Puppet::Application.superclass }
+      stub_const('Puppet::Application', Class.new(Puppet::Application))
     end
 
     it 'should set Puppet::Application.restart!' do

spec/unit/type/exec_spec.rb

@@ -266,15 +266,7 @@ def exec_stub(options = {})
 
     describe "on platforms where path separator is not :" do
       before :each do
-        @old_verbosity = $VERBOSE
-        $VERBOSE = nil
-        @old_separator = File::PATH_SEPARATOR
-        File::PATH_SEPARATOR = 'q'
-      end
-
-      after :each do
-        File::PATH_SEPARATOR = @old_separator
-        $VERBOSE = @old_verbosity
+        stub_const('File::PATH_SEPARATOR', 'q')
       end
 
       it "should use the path separator of the current platform" do

spec/unit/util/monkey_patches_spec.rb

@@ -12,10 +12,12 @@
       expect(Dir.exists?(__dir__)).to be true
     end
 
-    if RUBY_VERSION >= '3.2' 
+    if RUBY_VERSION >= '3.2'
       it 'logs a warning message' do
-        expect(Dir).to receive(:warn).with('exists? is a deprecated name, use exist? instead')
-        Dir.exists?(__dir__)
+        expect(Dir).to receive(:warn).with("Dir.exists?('#{__dir__}') is deprecated, use Dir.exist? instead")
+        with_verbose_enabled do
+          Dir.exists?(__dir__)
+        end
       end
     end
   end
@@ -33,8 +35,10 @@
 
     if RUBY_VERSION >= '3.2'
       it 'logs a warning message' do
-        expect(File).to receive(:warn).with('exists? is a deprecated name, use exist? instead')
-        File.exists?(__FILE__)
+        expect(File).to receive(:warn).with("File.exists?('#{__FILE__}') is deprecated, use File.exist? instead")
+        with_verbose_enabled do
+          File.exists?(__FILE__)
+        end
       end
     end
   end