Skip to content

Commit

Permalink
Run tests on TruffleRuby, all tests pass now (#2198)
Browse files Browse the repository at this point in the history
* Support skip_on :truffleruby

* Remove unused variable declaration

* Properly skip tests which need fork

* Improve NO_FORK_MSG

* Keep the Tempfile instances alive in test_redirect_io.rb

* Otherwise they could GC in the middle of the test, and the files could
  then be deleted.

* Use a better way to find a free port

* Read directly from the socket in #read_and_drop

* 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.

* TruffleRuby should pass the CI now, remove from allowed failures

* Use a timeout of 120 for all non-MRI implementations

* 60 doesn't seem enough in CI for TestThreadPool#test_trim on TruffleRuby.

* Fix check for cluster mode in integration tests

* Improve integration tests to fail more clearly if the pid file does not exist

* Make integration tests more robust

* Add skips for unreliable or racy tests

* Add ChangeLog entry

* No need to run RuboCop on non-MRI implementations

* This should speed up CI a bit for those jobs.
  • Loading branch information
eregon committed Mar 24, 2020
1 parent 3bf45fb commit 0b737cc
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 36 deletions.
13 changes: 8 additions & 5 deletions .github/workflows/ruby.yml
Expand Up @@ -18,7 +18,7 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu-18.04, macos ]
ruby: [ 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, head, jruby ]
ruby: [ 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, head, jruby, truffleruby-head ]

steps:
- name: repo checkout
Expand All @@ -42,9 +42,13 @@ jobs:
- name: compile
run: bundle exec rake compile

- name: rubocop
if: startsWith(matrix.ruby, '2.')
run: bundle exec rake rubocop

- name: test
timeout-minutes: 8
run: bundle exec rake
run: bundle exec rake test:all

win32:
name: >-
Expand Down Expand Up @@ -89,7 +93,7 @@ jobs:
timeout-minutes: 8
run: bundle exec rake

nonMRIHead:
allowedFailures:
name: >-
${{ matrix.cfg.os }}, ${{ matrix.cfg.ruby }}
env:
Expand All @@ -104,8 +108,7 @@ jobs:
fail-fast: false
matrix:
cfg:
- { os: ubuntu-latest, ruby: jruby-head }
- { os: ubuntu-latest, ruby: truffleruby-head }
- { os: ubuntu-latest, ruby: jruby-head }

steps:
- name: repo checkout
Expand Down
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -25,6 +25,7 @@
* Rescue IO::WaitReadable instead of EAGAIN for blocking read (#2121)
* Ensure `BUNDLE_GEMFILE` is unspecified in workers if unspecified in master when using `prune_bundler` (#2154)
* Rescue and log exceptions in hooks defined by users (on_worker_boot, after_worker_fork etc) (#1551)
* Read directly from the socket in #read_and_drop to avoid raising further SSL errors (#2198)

* Refactor
* Remove unused loader argument from Plugin initializer (#2095)
Expand Down
13 changes: 8 additions & 5 deletions lib/puma/minissl.rb
Expand Up @@ -125,11 +125,14 @@ def flush

def read_and_drop(timeout = 1)
return :timeout unless IO.select([@socket], nil, nil, timeout)
return :eof unless read_nonblock(1024)
:drop
rescue Errno::EAGAIN
# do nothing
:eagain
case @socket.read_nonblock(1024, exception: false)
when nil
:eof
when :wait_readable
:eagain
else
:drop
end
end

def should_drop_bytes?
Expand Down
17 changes: 6 additions & 11 deletions test/helper.rb
Expand Up @@ -43,15 +43,10 @@ def hit(uris)
end

module UniquePort
@port = 3211
@mutex = Mutex.new

def self.call
@mutex.synchronize {
@port += 1
@port = 3307 if @port == 3306 # MySQL on Actions
@port
}
TCPServer.open('127.0.0.1', 0) do |server|
server.connect_address.ip_port
end
end
end

Expand All @@ -62,7 +57,7 @@ class TestTookTooLong < Timeout::Error
end

def run(*)
::Timeout.timeout(Puma.jruby? ? 120 : 60, TestTookTooLong) { super }
::Timeout.timeout(RUBY_ENGINE == 'ruby' ? 60 : 120, TestTookTooLong) { super }
end
end

Expand All @@ -78,7 +73,7 @@ module TestSkips
# usage: skip NO_FORK_MSG unless HAS_FORK
# windows >= 2.6 fork is not defined, < 2.6 fork raises NotImplementedError
HAS_FORK = ::Process.respond_to? :fork
NO_FORK_MSG = "Kernel.fork isn't available on the #{RUBY_PLATFORM} platform"
NO_FORK_MSG = "Kernel.fork isn't available on #{RUBY_ENGINE} on #{RUBY_PLATFORM}"

# socket is required by puma
# usage: skip UNIX_SKT_MSG unless UNIX_SKT_EXIST
Expand All @@ -99,11 +94,11 @@ def skip_unless_signal_exist?(sig, bt: caller)
# optional suffix kwarg is appended to the skip message
# optional suffix bt should generally not used
def skip_on(*engs, suffix: '', bt: caller)
skip_msg = false
engs.each do |eng|
skip_msg = case eng
when :darwin then "Skipped on darwin#{suffix}" if RUBY_PLATFORM[/darwin/]
when :jruby then "Skipped on JRuby#{suffix}" if Puma.jruby?
when :truffleruby then "Skipped on TruffleRuby#{suffix}" if RUBY_ENGINE == "truffleruby"
when :windows then "Skipped on Windows#{suffix}" if Puma.windows?
when :ci then "Skipped on ENV['CI']#{suffix}" if ENV["CI"]
when :no_bundler then "Skipped w/o Bundler#{suffix}" if !defined?(Bundler)
Expand Down
6 changes: 3 additions & 3 deletions test/shell/run.rb
@@ -1,10 +1,10 @@
require "puma"
require "puma/detect"

TESTS_TO_RUN = if Puma.jruby?
%w[t1 t2]
else
TESTS_TO_RUN = if Process.respond_to?(:fork)
%w[t1 t2 t3]
else
%w[t1 t2]
end

results = TESTS_TO_RUN.map do |test|
Expand Down
6 changes: 3 additions & 3 deletions test/shell/t1.rb
@@ -1,8 +1,8 @@
system "ruby -rrubygems -Ilib bin/puma -p 10102 -C test/shell/t1_conf.rb test/rackup/hello.ru &"
sleep (defined?(JRUBY_VERSION) ? 7 : 5)
system "curl http://localhost:10102/"

system "kill `cat t1-pid`"
sleep 1 until system "curl http://localhost:10102/"

Process.kill :TERM, Integer(File.read("t1-pid"))

sleep 1

Expand Down
4 changes: 2 additions & 2 deletions test/shell/t2.rb
@@ -1,6 +1,6 @@
system "ruby -rrubygems -Ilib bin/pumactl -F test/shell/t2_conf.rb start &"
sleep (defined?(JRUBY_VERSION) ? 7 : 5)
system "curl http://localhost:10103/"

sleep 1 until system "curl http://localhost:10103/"

out=`ruby -rrubygems -Ilib bin/pumactl -F test/shell/t2_conf.rb status`

Expand Down
4 changes: 2 additions & 2 deletions test/shell/t3.rb
Expand Up @@ -3,13 +3,13 @@

worker_pid_was_present = File.file? "t3-worker-2-pid"

system "kill `cat t3-worker-2-pid`" # kill off a worker
Process.kill :TERM, Integer(File.read("t3-worker-2-pid")) # kill off a worker

sleep 2

worker_index_within_number_of_workers = !File.file?("t3-worker-3-pid")

system "kill `cat t3-pid`"
Process.kill :TERM, Integer(File.read("t3-pid"))

File.unlink "t3-pid" if File.file? "t3-pid"
File.unlink "t3-worker-0-pid" if File.file? "t3-worker-0-pid"
Expand Down
2 changes: 1 addition & 1 deletion test/test_integration_pumactl.rb
Expand Up @@ -20,7 +20,7 @@ def teardown
end

def test_stop_tcp
skip_on :jruby # Undiagnose thread race. TODO fix
skip_on :jruby, :truffleruby # Undiagnose thread race. TODO fix
@control_tcp_port = UniquePort.call
cli_server "-q test/rackup/sleep.ru --control-url tcp://#{HOST}:#{@control_tcp_port} --control-token #{TOKEN} -S #{@state_path}"

Expand Down
11 changes: 8 additions & 3 deletions test/test_redirect_io.rb
Expand Up @@ -7,15 +7,20 @@ class TestRedirectIO < TestIntegration
def setup
super

@out_file_path = Tempfile.new('puma-out').path
@err_file_path = Tempfile.new('puma-err').path
# Keep the Tempfile instances alive to avoid being GC'd
@out_file = Tempfile.new('puma-out')
@err_file = Tempfile.new('puma-err')
@out_file_path = @out_file.path
@err_file_path = @err_file.path
end

def teardown
super

paths = [@out_file_path, @err_file_path, @old_out_file_path, @old_err_file_path].compact
File.unlink(*paths)
@out_file = nil
@err_file = nil
end

def test_sighup_redirects_io_single
Expand Down Expand Up @@ -47,7 +52,7 @@ def test_sighup_redirects_io_single
end

def test_sighup_redirects_io_cluster
skip_on :jruby # Server isn't coming up in CI, TODO Fix
skip NO_FORK_MSG unless HAS_FORK
skip_unless_signal_exist? :HUP

cli_args = [
Expand Down
3 changes: 2 additions & 1 deletion test/test_thread_pool.rb
Expand Up @@ -64,7 +64,7 @@ def test_append_queues_on_max
end

def test_trim
skip_on :jruby # Undiagnose thread race. TODO fix
skip_on :jruby, :truffleruby # Undiagnose thread race. TODO fix
pool = new_pool(0, 1) do |work|
@work_mutex.synchronize do
@work_done.signal
Expand All @@ -87,6 +87,7 @@ def test_trim
end

def test_trim_leaves_min
skip_on :truffleruby # Undiagnose thread race. TODO fix
pool = new_pool(1, 2) do |work|
@work_mutex.synchronize do
@work_done.signal
Expand Down

0 comments on commit 0b737cc

Please sign in to comment.