Enable warning policy
teoljungberg opened this issue · 2 comments
In #211 we introduced a no warnings policy (and the commits following on master) - this was then disabled due to issues with older ruby versions. I came back to think about this today, and wondered if we should re-introduce it - and started to work on that.
However, Zeitwerk is now using warn - and when those cases are hit in the test the warnings helper catches it
I can rework the warning policy to allow warnings that come from within zeitwerk to be allowed, and disallow others. But before doing that, I wanted to check if we should invest in this or drop it.
Example:
- WARNING: Zeitwerk defines the constant Foo after the file
/home/teo/src/github.com/fxn/zeitwerk/test/tmp/lib/foo.rb
To prevent that, please configure the loader to ignore it:
loader.ignore("#{__dir__}/foo.rb")
Otherwise, there is a flag to silence this warning:
Zeitwerk::Loader.for_gem(warn_on_extra_files: false)
/home/teo/src/github.com/fxn/zeitwerk/test/support/no_warnings_policy.rb:14:in `warn'
<internal:warning>:51:in `warn'
/home/teo/src/github.com/fxn/zeitwerk/lib/zeitwerk/gem_loader.rb:49:in `block in warn_on_extra_files'
/home/teo/src/github.com/fxn/zeitwerk/lib/zeitwerk/loader/helpers.rb:40:in `block in ls'
/home/teo/src/github.com/fxn/zeitwerk/lib/zeitwerk/loader/helpers.rb:25:in `each'
/home/teo/src/github.com/fxn/zeitwerk/lib/zeitwerk/loader/helpers.rb:25:in `ls'
/home/teo/src/github.com/fxn/zeitwerk/lib/zeitwerk/gem_loader.rb:41:in `warn_on_extra_files'
/home/teo/src/github.com/fxn/zeitwerk/lib/zeitwerk/gem_loader.rb:31:in `setup'
/home/teo/src/github.com/fxn/zeitwerk/test/tmp/lib/my_gem.rb:3:in `<top (required)>'
With the following diff:
diff --git a/test/support/no_warnings_policy.rb b/test/support/no_warnings_policy.rb
index 710c5c0..5ced85d 100644
--- a/test/support/no_warnings_policy.rb
+++ b/test/support/no_warnings_policy.rb
@@ -1,17 +1,17 @@
# frozen_string_literal: true
module NoWarningsPolicy
- MESSAGE = "This test suite aborts on warnings, please fix the one above."
+ MESSAGE = "This test suite aborts on warnings, please fix the one above.\n\n - %<message>s"
if Warning.method(:warn).arity == 1
- def warn(*)
+ def warn(message)
super
- abort(MESSAGE)
+ raise(format(MESSAGE, message: message))
end
else
- def warn(*, **)
+ def warn(message, **)
super
- abort(MESSAGE)
+ raise(format(MESSAGE, message: message))
end
end
end
Hey, thanks for the followup on this topic.
I am 👍 on raising in the test suite, unless it is more trouble than what is worth. By now, it seems to be possible with little effort.
By researching this a bit I founded the warning gem, which encapsulates monkey patching Warning
, so our code here is a bit more clean.
I founded a warning issued in 2.5 that can be ignored, and wrote 7898292 with all this.
Let's have this enabled and see which is the experience.
Thanks!