Skip to content

Commit

Permalink
+ Added -Werror to raise on any warning output. (byroot)
Browse files Browse the repository at this point in the history
+ Added UnexpectedWarning as a failure summary type, added count to output if activated.

Also allows for -Wall or -W or -W<category>.
Fixed all tests to pass vanilla + -Werror.

Found (but not yet fixed) tests that currently fail vanilla if `$-w=nil`.

[git-p4: depot-paths = "//src/minitest/dev/": change = 14142]
  • Loading branch information
zenspider committed May 15, 2024
1 parent 609f1ad commit f0f17b9
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 39 deletions.
1 change: 1 addition & 0 deletions Manifest.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ lib/minitest/assertions.rb
lib/minitest/autorun.rb
lib/minitest/benchmark.rb
lib/minitest/compress.rb
lib/minitest/error_on_warning.rb
lib/minitest/expectations.rb
lib/minitest/hell.rb
lib/minitest/manual_plugins.rb
Expand Down
34 changes: 33 additions & 1 deletion lib/minitest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,20 @@ def self.process_args args = [] # :nodoc:
options[:skip] = s.chars.to_a
end

ruby27plus = ::Warning.respond_to?(:[]=)

opts.on "-W[error]", String, "Turn Ruby warnings into errors" do |s|
options[:Werror] = true
case s
when "error", "all", nil then
require "minitest/error_on_warning"
$VERBOSE = true
::Warning[:deprecated] = true if ruby27plus
else
::Warning[s.to_sym] = true if ruby27plus # check validity of category
end
end

unless extensions.empty?
opts.separator ""
opts.separator "Known extensions: #{extensions.join(", ")}"
Expand Down Expand Up @@ -782,6 +796,11 @@ class StatisticsReporter < Reporter

attr_accessor :errors

##
# Total number of tests that warned.

attr_accessor :warnings

##
# Total number of tests that where skipped.

Expand All @@ -797,6 +816,7 @@ def initialize io = $stdout, options = {} # :nodoc:
self.total_time = nil
self.failures = nil
self.errors = nil
self.warnings = nil
self.skips = nil
end

Expand Down Expand Up @@ -825,6 +845,7 @@ def report
self.total_time = Minitest.clock_time - start_time
self.failures = aggregate[Assertion].size
self.errors = aggregate[UnexpectedError].size
self.warnings = aggregate[UnexpectedWarning].size
self.skips = aggregate[Skip].size
end
end
Expand Down Expand Up @@ -900,6 +921,8 @@ def summary # :nodoc:
results.any?(&:skipped?) unless
options[:verbose] or options[:show_skips] or ENV["MT_NO_SKIP_MSG"]

extra.prepend ", %d warnings" % [warnings] if options[:Werror]

"%d runs, %d assertions, %d failures, %d errors, %d skips%s" %
[count, assertions, failures, errors, skips, extra]
end
Expand Down Expand Up @@ -1034,6 +1057,15 @@ def result_label # :nodoc:
end
end

##
# Assertion raised on warning when running in -Werror mode.

class UnexpectedWarning < Assertion
def result_label # :nodoc:
"Warning"
end
end

##
# Provides a simple set of guards that you can use in your tests
# to skip execution if it is not applicable. These methods are
Expand Down Expand Up @@ -1107,7 +1139,7 @@ def windows? platform = RUBY_PLATFORM

class BacktraceFilter

MT_RE = %r%lib/minitest% #:nodoc:
MT_RE = %r%lib/minitest|internal:warning% #:nodoc:

attr_accessor :regexp

Expand Down
11 changes: 11 additions & 0 deletions lib/minitest/error_on_warning.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Minitest

module ErrorOnWarning
def warn(message, category: nil)
message = "[#{category}] #{message}" if category
raise UnexpectedWarning, message
end
end

::Warning.singleton_class.prepend(ErrorOnWarning)
end
13 changes: 12 additions & 1 deletion test/minitest/metametameta.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,19 @@ def with_backtrace_filter filter
end
end
end
end

def error_on_warn?
defined?(Minitest::ErrorOnWarning)
end

def assert_deprecation re = /DEPRECATED/
assert_output "", re do
yield
end
rescue Minitest::UnexpectedWarning => e # raised if -Werror was used
assert_match re, e.message
end
end

class FakeNamedTest < Minitest::Test
@@count = 0
Expand Down
29 changes: 14 additions & 15 deletions test/minitest/test_minitest_assertions.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# encoding: UTF-8

require "minitest/autorun"
require_relative "metametameta"

if defined? Encoding then
e = Encoding.default_external
Expand Down Expand Up @@ -33,7 +34,6 @@ class TestMinitestAssertions < Minitest::Test

class DummyTest
include Minitest::Assertions
# include Minitest::Reportable # TODO: why do I really need this?

attr_accessor :assertions, :failure

Expand All @@ -58,15 +58,6 @@ def teardown
"expected #{@assertion_count} assertions to be fired during the test, not #{@tc.assertions}")
end

def assert_deprecated name
dep = /DEPRECATED: #{name}. From #{__FILE__}:\d+(?::.*)?/
dep = "" if $-w.nil?

assert_output nil, dep do
yield
end
end

def assert_triggered expected, klass = Minitest::Assertion
e = assert_raises klass do
yield
Expand Down Expand Up @@ -301,7 +292,7 @@ def test_assert_equal_does_not_allow_lhs_nil
err_re = /Use assert_nil if expecting nil from .*test_minitest_\w+.rb/
err_re = "" if $-w.nil?

assert_output "", err_re do
assert_deprecation err_re do
@tc.assert_equal nil, nil
end
end
Expand Down Expand Up @@ -975,16 +966,24 @@ def test_assert_same_triggered
end

def test_assert_send
assert_deprecated :assert_send do
@assertion_count = 0 if error_on_warn?
assert_deprecation(/DEPRECATED: assert_send/) do
@tc.assert_send [1, :<, 2]
end
end

def test_assert_send_bad
assert_deprecated :assert_send do
assert_triggered "Expected 1.>(*[2]) to return true." do
if error_on_warn? then
@assertion_count = 0
assert_deprecation(/DEPRECATED: assert_send/) do
@tc.assert_send [1, :>, 2]
end
else
assert_triggered "Expected 1.>(*[2]) to return true." do
assert_deprecation(/DEPRECATED: assert_send/) do
@tc.assert_send [1, :>, 2]
end
end
end
end

Expand Down Expand Up @@ -1502,7 +1501,7 @@ def test_skip_until
d0 = Time.now
d1 = d0 + 86_400 # I am an idiot

assert_output "", /Stale skip_until \"not yet\" at .*?:\d+$/ do
assert_deprecation(/Stale skip_until \"not yet\" at .*?:\d+$/) do
assert_skip_until d0, "not yet"
end

Expand Down
4 changes: 3 additions & 1 deletion test/minitest/test_minitest_mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,12 @@ def test_mock_block_is_passed_keyword_args__args__old_style_both
arg1, arg2, arg3 = :bar, [1, 2, 3], { :a => "a" }
mock = Minitest::Mock.new

assert_output nil, /Using MT_KWARGS_HAC. yet passing kwargs/ do
assert_deprecation(/Using MT_KWARGS_HAC. yet passing kwargs/) do
mock.expect :foo, nil, [{}], k1: arg1, k2: arg2, k3: arg3
end

skip "-Werror" if error_on_warn? # mock above raised, so this is dead

mock.foo({}, k1: arg1, k2: arg2, k3: arg3)

assert_mock mock
Expand Down
36 changes: 19 additions & 17 deletions test/minitest/test_minitest_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -284,18 +284,14 @@ def bad_pattern
end

it "needs to warn on equality with nil" do
@assertion_count += 1 # extra test
@assertion_count = 3
@assertion_count += 2 unless error_on_warn? # 2 extra assertions

exp = /DEPRECATED: Use assert_nil if expecting nil from .* This will fail in Minitest 6./

out, err = capture_io do
assert_deprecation exp do
assert_success _(nil).must_equal(nil)
end

exp = "DEPRECATED: Use assert_nil if expecting nil from #{__FILE__}:#{__LINE__-3}. " \
"This will fail in Minitest 6.\n"
exp = "" if $-w.nil?

assert_empty out
assert_equal exp, err
end

it "needs to verify floats outside a delta" do
Expand Down Expand Up @@ -576,7 +572,8 @@ def bad_pattern

it "can NOT use must_equal in a thread. It must use expect in a thread" do
skip "N/A" if ENV["MT_NO_EXPECTATIONS"]
assert_raises RuntimeError do

assert_raises RuntimeError, Minitest::UnexpectedWarning do
capture_io do
Thread.new { (1 + 1).must_equal 2 }.join
end
Expand All @@ -586,9 +583,9 @@ def bad_pattern
it "fails gracefully when expectation used outside of `it`" do
skip "N/A" if ENV["MT_NO_EXPECTATIONS"]

@assertion_count += 1
@assertion_count += 2 # assert_match is compound

e = assert_raises RuntimeError do
e = assert_raises RuntimeError, Minitest::UnexpectedWarning do
capture_io do
Thread.new { # forces ctx to be nil
describe("woot") do
Expand All @@ -598,17 +595,21 @@ def bad_pattern
end
end

assert_equal "Calling #must_equal outside of test.", e.message
exp = "Calling #must_equal outside of test."
exp = "DEPRECATED: global use of must_equal from" if error_on_warn?

assert_match exp, e.message
end

it "deprecates expectation used without _" do
skip "N/A" if ENV["MT_NO_EXPECTATIONS"]

@assertion_count += 3
@assertion_count += 1
@assertion_count += 2 unless error_on_warn?

exp = /DEPRECATED: global use of must_equal from/

assert_output "", exp do
assert_deprecation exp do
(1 + 1).must_equal 2
end
end
Expand All @@ -618,12 +619,13 @@ def bad_pattern
it "deprecates expectation used without _ with empty backtrace_filter" do
skip "N/A" if ENV["MT_NO_EXPECTATIONS"]

@assertion_count += 3
@assertion_count += 1
@assertion_count += 2 unless error_on_warn?

exp = /DEPRECATED: global use of must_equal from/

with_empty_backtrace_filter do
assert_output "", exp do
assert_deprecation exp do
(1 + 1).must_equal 2
end
end
Expand Down
8 changes: 4 additions & 4 deletions test/minitest/test_minitest_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1126,19 +1126,19 @@ def test_jruby_eh
end

def test_rubinius_eh
assert_output "", /DEPRECATED/ do
assert_deprecation do
assert self.class.rubinius? "rbx"
end
assert_output "", /DEPRECATED/ do
assert_deprecation do
assert self.rubinius? "rbx"
end
end

def test_maglev_eh
assert_output "", /DEPRECATED/ do
assert_deprecation do
assert self.class.maglev? "maglev"
end
assert_output "", /DEPRECATED/ do
assert_deprecation do
assert self.maglev? "maglev"
end
end
Expand Down

0 comments on commit f0f17b9

Please sign in to comment.