Add JRuby and TruffleRuby to CI

[nevans]

_I'd like to add CI for JRuby and TruffleRuby, but the last time I tried I got a bunch of errors (admittedly, I didn't spend much time trying to figure them out).

Ideally, we could fix the errors without significant changes, and net-imap could be fully functional under those implementations.

If the changes needed for full compatibility are significant, maybe we can just mark some tests as pending. That will at least serve as a warning of what's incompatible, and prevent new changes from inadvertently widening that gap (see for example #452).

Fixed by

• (replaced) ✅ Add TruffleRuby and JRuby to CI #470
• ✅ Add TruffleRuby to CI #528
• ✅🚧 Add JRuby to CI #529

Other related issues

net-imap issues/PRs

• ♻️ Short-circuit frozen SequenceSet modifications #473 (JRuby FrozenError doesn't use #inspect)
• ✅ Update Regexp.linear_time? tests for non-CRuby #477
• ✅ Update ResponseReader, UIDFetchData, DeprecatedClientOptions tests #476
• ✅ Add timeouts to CI workflow #478
• 🐛 Fix loading of net/imap for JRuby/TruffleRuby #530
• ✅ Test overriding inherited ::Data methods #531
• ✅ Fix flaky tests with FakeServer#Connection#close mutex #520
• Don't wait for lock before closing the socket in #disconnect

JRuby issues/PRs

• Data fixes and improvements jruby/jruby#8829 - to fix Data subclasses
• Splat with multi-line expression improperly expands jruby/jruby#8830 - fixed the response parser tests
• Prism compiler fails with lonely operator in complex call sequence jruby/jruby#8832
• JRuby isn't escaping strings correctly json#870
• Struct inheritance issues: 002db9f.
• All (or nearly all) of the SSL connection tests are failing. Hopefully this only requires small tweaks to net-imap test code.

TruffleRuby issues/PRs:

• "%p" % nil is "nil" on CRuby but "" on TruffleRuby truffleruby/truffleruby#3846
• Recursive inspect for Data gives StackOverflowError truffleruby/truffleruby#3847
• Frozen Struct can be modified truffleruby/truffleruby#3850
• Incompatible Struct#initialize with mixed keyword args truffleruby/truffleruby#3855
• StringIO#gets("\r\n", 10) returns an extra \n truffleruby/truffleruby#3856
• Support the same Regexps to be Regexp.linear_time? as CRuby truffleruby/truffleruby#3858
• Can't override ::Data methods in abstract class truffleruby/truffleruby#3892

Misc

• I either omitted or marked as pending everything that's currently failing.
• TODO: diagnose deadlock on error (between get_tagged_response and receive_responses?)
• Both JRuby and TruffleRuby still fail in various ways when inheriting from Data.
  • I temporarily added in a crazy feature-check to our Data polyfill. Rather than just check RUBY_ENGINE and RUBY_ENGINE_VERSION and defined?(::Data) and ::Data.respond_to?(:define), it tries to use all of the features that were causing tests to fail. If anything looks off, it prints a warning and falls back to the polyfill. I'm not going to merge this feature check. But it's in the branch. 😉
  • net-imap v0.6.0 will still be deleting DataLite.

[nevans]

Note: I rebased my previous work https://github.com/ruby/net-imap/tree/ci-alt-ruby-impls and tried a few more things. But TruffleRuby still has 9 failures, and JRuby isn't even successfully running the tests yet.

[eregon]

It would be great indeed.
In general we're trying to add truffleruby to all default & bundled gems CI (truffleruby/truffleruby#2644) to avoid breakage and regressions like #452.

Regarding the failures, I think it's always better to run all tests except these 9 known failures, than to test nothing.
So I would suggest to add TruffleRuby in CI and omit these 9 failures for now.
It'd be great to have the output of the failures somewhere so we can look if there are some which are easy to fix.

@nevans Could you do that since you already have that branch?
If you would like some help (e.g. we make the PR based on your branch), @andrykonchin or I can help, let us know what you prefer.

[headius]

@nevans I'll have a look at failures a bit today.

[headius]

The "crash" appears to be something missing in JRuby 10's Data impl. Interesting, since we pass all tests and specs related to Data currently.

[headius]

The crashing was all due to some missing features of Data: subclass behavior and empty definitions (no fields). I've fixed those in jruby/jruby#8829.

The specs run now, but hang because the "fake server" implementation does not appear to work properly on JRuby. Most of those tests, the server terminates early. Some of them do not handle this well and end up hanging waiting forever for a server that has gone away.

I'll keep poking at it.

[headius]

I'm making some progress but it was complicated by a deadlock in the net-imap tests. I've

There seems to be a bug in JRuby where the response name is not getting set right:

#<struct Net::IMAP::TaggedResponse tag="RUBY0001", name=" ", data=["OK", #<struct Net::IMAP::ResponseText code=nil, text="LOGOUT done">], raw_data="RUBY0001 OK LOGOUT done\r\n">

When this is handled by get_tagged_response, it falls off the end of the case and tries to force a disconnect:

net-imap/lib/net/imap.rb

Line 3457 in f76d433

disconnect

Disconnect tries to close down the socket and then wait for the receiver thread to exit. When the socket gets closed down, the receiver thread proceeds to try to call state_logout!:

net-imap/lib/net/imap.rb

Line 3350 in f76d433

state_logout!

Unfortunately both send_command (which calls get_tagged_response) and state_logout! call synchronize on the same object, which means the receiver thread gets stuck waiting for the lock that the main thread is still holding.

It does not affect the normal exit from get_tagged_response because it doesn't try to force a disconnect before leaving the synchronize. The receiver thread gets shut down normally elsewhere.

Obviously the bug in constructing the response needs to be fixed in JRuby, but this code shouldn't deadlock when the server's response is not in the expected valid responses.