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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -24,6 +24,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
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.

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|
Copy link
Member

Choose a reason for hiding this comment

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

👏

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