From 1d8cf64722d8a7529f7cd205be5f16a89b7a67fd Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Wed, 1 Mar 2023 13:15:43 +0100 Subject: [PATCH] Remove Warning.warn override in MSpec and --warnings option * 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. --- lib/mspec/commands/mspec.rb | 5 ---- lib/mspec/helpers/io.rb | 4 ++-- lib/mspec/matchers/complain.rb | 2 -- lib/mspec/utils/warnings.rb | 43 ---------------------------------- spec/commands/mspec_spec.rb | 27 --------------------- 5 files changed, 2 insertions(+), 79 deletions(-) diff --git a/lib/mspec/commands/mspec.rb b/lib/mspec/commands/mspec.rb index 9c38cebc..f5341c69 100755 --- a/lib/mspec/commands/mspec.rb +++ b/lib/mspec/commands/mspec.rb @@ -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 diff --git a/lib/mspec/helpers/io.rb b/lib/mspec/helpers/io.rb index 29c6c37a..2ad14f47 100644 --- a/lib/mspec/helpers/io.rb +++ b/lib/mspec/helpers/io.rb @@ -7,7 +7,7 @@ def initialize end def write(*str) - self << str.join + self << str.join('') end def << str @@ -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) diff --git a/lib/mspec/matchers/complain.rb b/lib/mspec/matchers/complain.rb index 887e72b4..19310c0b 100644 --- a/lib/mspec/matchers/complain.rb +++ b/lib/mspec/matchers/complain.rb @@ -19,7 +19,6 @@ 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 @@ -27,7 +26,6 @@ def matches?(proc) ensure $VERBOSE = @verbose $stderr = @saved_err - Thread.current[:in_mspec_complain_matcher] = false end @warning = err.to_s diff --git a/lib/mspec/utils/warnings.rb b/lib/mspec/utils/warnings.rb index 0d3d36fa..23efc696 100644 --- a/lib/mspec/utils/warnings.rb +++ b/lib/mspec/utils/warnings.rb @@ -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 diff --git a/spec/commands/mspec_spec.rb b/spec/commands/mspec_spec.rb index 82201c20..d19bebb2 100644 --- a/spec/commands/mspec_spec.rb +++ b/spec/commands/mspec_spec.rb @@ -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