Skip to content

Commit

Permalink
Remove Warning.warn override in MSpec and --warnings option
Browse files Browse the repository at this point in the history
* Overriding Warning.warn cause several problems, including it's much
  harder and confusing to test Warning.warn in specs. Notably caused by
  Warning.warn in 2.7 not supporting the category: keyword argument.
* specs fail when $VERBOSE is true globally, so there is no real value
  to filter those $VERBOSE=true-only warnings.
* mspec requires ruby 2.7+, which has Warning.warn, so
  `$VERBOSE = nil unless ENV['OUTPUT_WARNINGS']` was dead code,
  and so the --warnings option had no effect.
  • Loading branch information
eregon committed Apr 25, 2023
1 parent 1346122 commit 1d8cf64
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 79 deletions.
5 changes: 0 additions & 5 deletions lib/mspec/commands/mspec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ def options(argv = ARGV)

options.targets

options.on("--warnings", "Don't suppress warnings") do
config[:flags] << '-w'
ENV['OUTPUT_WARNINGS'] = '1'
end

options.on("-j", "--multi", "Run multiple (possibly parallel) subprocesses") do
config[:multi] = true
end
Expand Down
4 changes: 2 additions & 2 deletions lib/mspec/helpers/io.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def initialize
end

def write(*str)
self << str.join
self << str.join('')
end

def << str
Expand All @@ -16,7 +16,7 @@ def << str
end

def print(*str)
write(str.join + $\.to_s)
write(str.join('') + $\.to_s)
end

def method_missing(name, *args, &block)
Expand Down
2 changes: 0 additions & 2 deletions lib/mspec/matchers/complain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@ def matches?(proc)
@verbose = $VERBOSE
err = IOStub.new

Thread.current[:in_mspec_complain_matcher] = true
$stderr = err
$VERBOSE = @options.key?(:verbose) ? @options[:verbose] : false
begin
proc.call
ensure
$VERBOSE = @verbose
$stderr = @saved_err
Thread.current[:in_mspec_complain_matcher] = false
end

@warning = err.to_s
Expand Down
43 changes: 0 additions & 43 deletions lib/mspec/utils/warnings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,3 @@
Warning[:deprecated] = true
Warning[:experimental] = false
end

if Object.const_defined?(:Warning) and Warning.respond_to?(:warn)
def Warning.warn(message, category: nil)
# Suppress any warning inside the method to prevent recursion
verbose = $VERBOSE
$VERBOSE = nil

if Thread.current[:in_mspec_complain_matcher]
return $stderr.write(message)
end

case message
# $VERBOSE = true warnings
when /(.+\.rb):(\d+):.+possibly useless use of (<|<=|==|>=|>) in void context/
# Make sure there is a .should otherwise it is missing
line_nb = Integer($2)
unless File.exist?($1) and /\.should(_not)? (<|<=|==|>=|>)/ === File.readlines($1)[line_nb-1]
$stderr.write message
end
when /possibly useless use of (\+|-) in void context/
when /assigned but unused variable/
when /method redefined/
when /previous definition of/
when /instance variable @.+ not initialized/
when /statement not reached/
when /shadowing outer local variable/
when /setting Encoding.default_(in|ex)ternal/
when /unknown (un)?pack directive/
when /(un)?trust(ed\?)? is deprecated/
when /\.exists\? is a deprecated name/
when /Float .+ out of range/
when /passing a block to String#(bytes|chars|codepoints|lines) is deprecated/
when /core\/string\/modulo_spec\.rb:\d+: warning: too many arguments for format string/
when /regexp\/shared\/new_ascii(_8bit)?\.rb:\d+: warning: Unknown escape .+ is ignored/
else
$stderr.write message
end
ensure
$VERBOSE = verbose
end
else
$VERBOSE = nil unless ENV['OUTPUT_WARNINGS']
end
27 changes: 0 additions & 27 deletions spec/commands/mspec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,33 +92,6 @@
end
end

RSpec.describe "The --warnings option" do
before :each do
@options, @config = new_option
allow(MSpecOptions).to receive(:new).and_return(@options)
@script = MSpecMain.new
allow(@script).to receive(:config).and_return(@config)
end

it "is enabled by #options" do
allow(@options).to receive(:on)
expect(@options).to receive(:on).with("--warnings", an_instance_of(String))
@script.options
end

it "sets flags to -w" do
@config[:flags] = []
@script.options ["--warnings"]
expect(@config[:flags]).to include("-w")
end

it "set OUTPUT_WARNINGS = '1' in the environment" do
ENV['OUTPUT_WARNINGS'] = '0'
@script.options ["--warnings"]
expect(ENV['OUTPUT_WARNINGS']).to eq('1')
end
end

RSpec.describe "The -j, --multi option" do
before :each do
@options, @config = new_option
Expand Down

0 comments on commit 1d8cf64

Please sign in to comment.