Fix compatibility with Ruby HEAD & add Ruby versions to CI

[sambostock]

Compatibility with Ruby HEAD is broken due to changes in ruby/logger#85.

class Logger
  def level
    @level_override[Fiber.current] || https://github.com/Level?type=source
    #              ^^^^^^^^^^^^^^^ undefined method `[]' for nil:NilClass (NoMethodError)
  end
end

See resque/resque#1856 for example.

ruby/logger#85 introduced setting @level_override in initialize. Since we don't call super, we need to set it ourselves.

This also adds Ruby 3.1, 3.2, & HEAD to the CI matrix.

[sambostock]

I'm not sure if test/mri_logger_test.rb should be updated. I assume it was adapted from Ruby's test/logger/test_logger.rb long ago, but has since become stale.

[jeremyevans]

IMO, MonoLogger#initialize should be changed to call super. Any other approach and the same issue will happen again if Logger#initialize sets another instance variable. In general, you must call super in initialize unless you control the parent class and are sure it is safe not to (or the parent class is Object or BasicObject).

[sambostock]

Yeah, I'm inclined to agree. I just went with this approach because it matches the current one, and I was unsure if there was a reason super isn't being called, and that would be a larger refactor. Would be good to hear from @steveklabnik about this.

[sambostock]

Okay, I looked into it, and I think if we just call super(nil) and follow up by setting @logdev we keep the same behavior without hard coding it.

[steveklabnik]

I was unsure if there was a reason super isn't being called, and that would be a larger refactor.

I wrote this gem almost a decade ago (one month left!) in an evening. If there was a reason, I don't remember why, and even if there was, stuff has probably changed in the meantime. This change sounds good to me.

[steveklabnik]

Hm, I re-ran these failures and they're still persisting. Seems like a coveralls issue; I'm not particularly tied to coveralls in general, just seemed like good hygiene at the time. What do either of you think? I'm tempted to just say "let's rip it out."

[sambostock]

Does Coveralls provide any information about the coverage against the logger API, or only this gem's patches?

[sambostock]

I think Coveralls already does the setup, so we just need to stop calling start ourselves, and pass the block to wear instead.

I can look into it later.

https://github.com/steveklabnik/mono_logger/blob/master/test/mri_logger_test.rb#L3-L7

[sambostock]

@steveklabnik I've pushed another commit that seems to fix the double registration problem.

[steveklabnik]

Thank you! Two things I'm noticing now that I'm re-running this:

1. it seems that this is trying to submit to a v1 of the coveralls API, which no longer exists. Personally im just in favor of removing coveralls at this point, there's no reason for any of us to waste more time on this. That said if you want to figure that out, I'm not gonna complain, but I don't want to suggest that you should do that.
2. it seems that ruby 3.1 and newer have an extra space at the end of the string, so the regex doesn't match, so the real test fails. I have not diagnosed why that is, but if you don't check it out, I am happy to at some point soon.

Really appreciate all the help here.

[sambostock]

I'm fine with removing Coveralls; I don't know if anyone else watching this PR has opinions.

I've been trying to figure out the answer to why the format no longer matches.

[steveklabnik]

I was struggling with this, so I asked @indirect. And he found it.

according to the Ruby 3.1.0 changelog, they updated to Logger 1.5.0
oh look! changes to datetime formatting

ruby/logger#60

Notice the trailing space for the value of datetime.

The only thing that's confusing here is, this patch removes the space, but this is the only versions we're actually seeing with the space? That's very confusing?

[steveklabnik]

@indirect beat me to it again:

https://github.com/ruby/logger/pull/60/files#diff-85fd2b8ea397f0dd33e3c2def70f714ce7b50f53485b42bef3003933de4e70afL16-R16

this line adds it. Our test code is taken from the previous, old version of the test code, this new version adds it in.

Sounds like we should call strip in the tests and call it a day.

[sambostock]

@steveklabnik I've

• used strip in the tests so we don't have to change them to be compatible across Ruby/logger versions
• I've removed Coveralls

If you can trigger CI to double check, and it passes, I can clean up the commits and then this should be good to go!

[steveklabnik]

Looking great! Happy to click merge once you're happy with the commits :) Thank you so much again for all the help here.

[sambostock]

@steveklabnik assuming CI still passes, it should be good to go now.

[st0012]

@sambostock Thank you for the fix 🙌

[yahonda]

Would you consider to release the newer version of mono_logger that includes this fix? Because Rails CI also runs Ruby 3.3.0dev that is failing as explained in resque/resque#1856

[steveklabnik]

@yahonda released as 1.1.2!

[yahonda]

Thanks for the new release.