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 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.
eregon added 7 commits Mar 22, 2020
* 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|

This comment has been minimized.

Copy link
@nateberkopec
@eregon eregon force-pushed the eregon:run-tests-on-truffleruby branch from 4af1bda to 122a1ee Mar 24, 2020
* This should speed up CI a bit for those jobs.
@eregon eregon force-pushed the eregon:run-tests-on-truffleruby branch from 122a1ee to 646d73a Mar 24, 2020
@nateberkopec
Copy link
Member

nateberkopec commented Mar 24, 2020

Thanks so much @eregon and Shopify! 🎉

@nateberkopec nateberkopec merged commit 0b737cc into puma:master Mar 24, 2020
27 checks passed
27 checks passed
build
Details
ubuntu-18.04, 2.2
Details
ubuntu-18.04, 2.3
Details
ubuntu-18.04, 2.4
Details
ubuntu-18.04, 2.5
Details
ubuntu-18.04, 2.6
Details
ubuntu-18.04, 2.7
Details
ubuntu-18.04, head
Details
ubuntu-18.04, jruby
Details
ubuntu-18.04, truffleruby-head
Details
macos, 2.2
Details
macos, 2.3
Details
macos, 2.4
Details
macos, 2.5
Details
macos, 2.6
Details
macos, 2.7
Details
macos, head
Details
macos, jruby
Details
macos, truffleruby-head
Details
windows-latest, 2.2
Details
windows-latest, 2.3
Details
windows-latest, 2.4
Details
windows-latest, 2.5
Details
windows-latest, 2.6
Details
windows-latest, 2.7
Details
windows-latest, mingw
Details
ubuntu-latest, jruby-head ubuntu-latest, jruby-head
Details
:eagain
else
:drop
end

This comment has been minimized.

Copy link
@eregon

eregon Mar 29, 2020

Author Contributor

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
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.