Enable warning policy

[teoljungberg]

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

https://github.com/fxn/zeitwerk/blob/180d2f6354c8548954763f9e83c9e629888e9f5b/lib/zeitwerk/gem_loader.rb#L49

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.

[teoljungberg]

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

[fxn]

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 7898292d5d6e7af2f0da6f95c793a269705de508 with all this.

Let's have this enabled and see which is the experience.

Thanks!