Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run tests on TruffleRuby, all tests pass now #2198

Merged
merged 15 commits into from Mar 24, 2020

Conversation

eregon
Copy link
Contributor

@eregon eregon commented Mar 23, 2020

Description

This PR improves the reliability of tests and as a result all tests now pass on TruffleRuby.
One extra fix was needed in TruffleRuby (oracle/truffleruby@2e046e4).
A major part of getting TruffleRuby to pass Puma's tests was by done by Shopify contributors to TruffleRuby (especially the signal stuff).

Since tests pass on TruffleRuby, I also moved TruffleRuby along with other tested versions in CI, so failures are no longer ignored.
I ran the CI twice in GitHub Actions and it passed in both cases.

cc @chrisseaton @nateberkopec

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • All new and existing tests passed, including Rubocop.

* Otherwise they could GC in the middle of the test, and the files could
  then be deleted.
* There is no point to decode the bytes since we are closing the socket
  in Puma::MiniSSL::Socket#close.
* Also, calling #engine_read_all might cause further SSL errors, which
  could hide the first SSL error. This notably happens in
  TestPumaServerSSLClient#test_verify_fail_if_no_client_cert
  if the server is faster than the client. The error in that case is
  "System error: Success - 0 (Puma::MiniSSL::SSLError)" which is not
  actually an error, but there is also nothing to read further from SSL.
Rakefile Outdated Show resolved Hide resolved
@port = 3307 if @port == 3306 # MySQL on Actions
@port
}
TCPServer.open('127.0.0.1', 0) do |server|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

* This should speed up CI a bit for those jobs.
@nateberkopec
Copy link
Member

Thanks so much @eregon and Shopify! 🎉

@nateberkopec nateberkopec merged commit 0b737cc into puma:master Mar 24, 2020
:eagain
else
:drop
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, this might be related to a bug in OpenSSL, mentioned in ruby/openssl#355 (comment)
Anyway, it seems safer to not call to SSL anymore if we are in such a state, as this commit does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants