Skip to content

Commit ff1880c

Browse files
committed
(PUP-11974) Only warn when Ruby's verbose is enabled
In ruby 2.7.x, the rb_file_exists_p and rb_dir_exists_p methods call rb_warning to log that the methods are deprecated[1][2] However, the rb_warning method is a noop if $VERBOSE is nil or false[3] To preserve the same behavior, only warn when $VERBOSE is truthy. Also include the class name and path in the warning so we can identify and fix the issue. [1] https://github.com/ruby/ruby/blob/1f4d4558484b370999954f3ede7e3aa3a3a01ef3/file.c#L1819 [2] https://github.com/ruby/ruby/blob/1f4d4558484b370999954f3ede7e3aa3a3a01ef3/dir.c#L3301 [3] https://github.com/ruby/ruby/blob/1f4d4558484b370999954f3ede7e3aa3a3a01ef3/error.c#L336-L338
1 parent 6f19bb8 commit ff1880c

File tree

4 files changed

+23
-17
lines changed

4 files changed

+23
-17
lines changed

lib/puppet/util/monkey_patches.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def daemonize
3333
unless Dir.singleton_methods.include?(:exists?)
3434
class Dir
3535
def self.exists?(file_name)
36-
warn('exists? is a deprecated name, use exist? instead')
36+
warn("Dir.exists?('#{file_name}') is deprecated, use Dir.exist? instead") if $VERBOSE
3737
Dir.exist?(file_name)
3838
end
3939
end
@@ -42,7 +42,7 @@ def self.exists?(file_name)
4242
unless File.singleton_methods.include?(:exists?)
4343
class File
4444
def self.exists?(file_name)
45-
warn('exists? is a deprecated name, use exist? instead')
45+
warn("File.exists?('#{file_name}') is deprecated, use File.exist? instead") if $VERBOSE
4646
File.exist?(file_name)
4747
end
4848
end

spec/lib/puppet_spec/verbose.rb

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1-
# Support code for running stuff with warnings disabled.
1+
# Support code for running stuff with warnings disabled or enabled
22
module Kernel
33
def with_verbose_disabled
44
verbose, $VERBOSE = $VERBOSE, nil
55
result = yield
66
$VERBOSE = verbose
77
return result
88
end
9+
10+
def with_verbose_enabled
11+
verbose, $VERBOSE = $VERBOSE, true
12+
begin
13+
yield
14+
ensure
15+
$VERBOSE = verbose
16+
end
17+
end
918
end

spec/unit/agent_spec.rb

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,12 @@ def stop
1515
end
1616
end
1717

18-
def without_warnings
19-
flag = $VERBOSE
20-
$VERBOSE = nil
21-
yield
22-
$VERBOSE = flag
23-
end
24-
2518
describe Puppet::Agent do
2619
before do
2720
@agent = Puppet::Agent.new(AgentTestClient, false)
2821

2922
# make Puppet::Application safe for stubbing; restore in an :after block; silence warnings for this.
30-
without_warnings { Puppet::Application = Class.new(Puppet::Application) }
23+
with_verbose_disabled { Puppet::Application = Class.new(Puppet::Application) }
3124
allow(Puppet::Application).to receive(:clear?).and_return(true)
3225
Puppet::Application.class_eval do
3326
class << self
@@ -44,7 +37,7 @@ def controlled_run(&block)
4437

4538
after do
4639
# restore Puppet::Application from stub-safe subclass, and silence warnings
47-
without_warnings { Puppet::Application = Puppet::Application.superclass }
40+
with_verbose_disabled { Puppet::Application = Puppet::Application.superclass }
4841
end
4942

5043
it "should set its client class at initialization" do

spec/unit/util/monkey_patches_spec.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
expect(Dir.exists?(__dir__)).to be true
1313
end
1414

15-
if RUBY_VERSION >= '3.2'
15+
if RUBY_VERSION >= '3.2'
1616
it 'logs a warning message' do
17-
expect(Dir).to receive(:warn).with('exists? is a deprecated name, use exist? instead')
18-
Dir.exists?(__dir__)
17+
expect(Dir).to receive(:warn).with("Dir.exists?('#{__dir__}') is deprecated, use Dir.exist? instead")
18+
with_verbose_enabled do
19+
Dir.exists?(__dir__)
20+
end
1921
end
2022
end
2123
end
@@ -33,8 +35,10 @@
3335

3436
if RUBY_VERSION >= '3.2'
3537
it 'logs a warning message' do
36-
expect(File).to receive(:warn).with('exists? is a deprecated name, use exist? instead')
37-
File.exists?(__FILE__)
38+
expect(File).to receive(:warn).with("File.exists?('#{__FILE__}') is deprecated, use File.exist? instead")
39+
with_verbose_enabled do
40+
File.exists?(__FILE__)
41+
end
3842
end
3943
end
4044
end

0 commit comments

Comments
 (0)