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.

Sign up for GitHub

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

Jump to bottom

Ensure integration tests runs with Puma #1874

Closed
dentarg opened this issue Feb 11, 2023 · 5 comments · Fixed by #1876
Closed

Ensure integration tests runs with Puma #1874

dentarg opened this issue Feb 11, 2023 · 5 comments · Fixed by #1876
Assignees
@dentarg
Labels
ci / testing

Comments

@dentarg
Copy link
Member

dentarg commented Feb 11, 2023

Tests output show puma is not installed, skipping integration tests so I think this require is not working as expected.

Originally posted by @epergo in #1858 (comment)
@dentarg dentarg self-assigned this Feb 11, 2023
@dentarg
Copy link
Member Author

dentarg commented Feb 11, 2023

So the reason for the change over at https://github.com/sinatra/sinatra/pull/1858/files/a8ec48da5861970a8dc97706d511093a00417ca7#diff-cb4a09a087ed85c2331c53261acfe2b38823d8acbdcaae0d0ec8cc7d499d04e9 (which is incorrect, should have been rack/handler/puma) was to battle this failure:

  1) Failure:
IntegrationTest#test_with_puma_starts_the_correct_server_1 [/app/test/integration_test.rb:52]:
Expected /
      ==\sSinatra\s\(v3.0.5\)\s
      has\staken\sthe\sstage\son\s\d+\sfor\sdevelopment\s
      with\sbackup\sfrom\s#<IntegrationHelper::BaseServer:0x0000aaaaf20a62a0>
    /ix to match "".

It happens because server.log is the empty (string) at

sinatra/test/integration_test.rb

Line 50 in 72e5b33

assert_match exp, server.log unless server.net_http_server? || server.rainbows?

when the async test runs

sinatra/test/integration_async_helper.rb

Line 7 in baa6bf7

%w(rainbows puma).each_with_index do |server_name, index|

It is empty for rainbows too, but as seen above, the assertion isn't run for rainbows. However, that conditional makes it so the assertions isn't run for rainbows for the regular integration tests as well, so I don't like the idea of adding such conditional for puma.

@dentarg
Copy link
Member Author

dentarg commented Feb 11, 2023

The reason for the log being empty seems to be this

IntegrationTest#test_with_puma_starts_the_correct_server_0 = log Exception e=Traceback (most recent call last):
	27: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:83:in `block in autorun'
	26: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:159:in `run'
	25: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:182:in `__run'
	24: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:182:in `map'
	23: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:182:in `block in __run'
	22: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:350:in `run'
	21: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:378:in `with_info_handler'
	20: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:391:in `on_signal'
	19: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:351:in `block in run'
	18: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:351:in `each'
	17: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:352:in `block (2 levels) in run'
	16: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:365:in `run_one_method'
	15: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:1051:in `run_one_method'
	14: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest/test.rb:95:in `run'
	13: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest/test.rb:247:in `with_info_handler'
	12: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:391:in `on_signal'
	11: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest/test.rb:96:in `block in run'
	10: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest.rb:296:in `time_it'
	 9: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest/test.rb:97:in `block (2 levels) in run'
	 8: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest/test.rb:199:in `capture_exceptions'
	 7: from /usr/local/bundle/gems/minitest-5.17.0/lib/minitest/test.rb:102:in `block (3 levels) in run'
	 6: from /app/test/integration_helper.rb:117:in `block (2 levels) in it'
	 5: from /app/test/integration_helper.rb:104:in `run_test'
	 4: from /app/test/integration_helper.rb:104:in `instance_eval'
	 3: from /app/test/integration_test.rb:49:in `block in <class:IntegrationTest>'
	 2: from /app/sinatra-contrib/lib/sinatra/runner.rb:96:in `log'
	 1: from /app/sinatra-contrib/lib/sinatra/runner.rb:96:in `loop'
/app/sinatra-contrib/lib/sinatra/runner.rb:96:in `block in log': undefined method `read_nonblock' for nil:NilClass (NoMethodError)
server.log=""

(the above output came from logging the #full_message of the exception at

sinatra/sinatra-contrib/lib/sinatra/runner.rb

Lines 91 to 96 in 72e5b33

def log
@log ||= ""
loop { @log << pipe.read_nonblock(1) }
rescue Exception
@log
end
)

@dentarg
Copy link
Member Author

dentarg commented Feb 12, 2023

No problem when running only async tests (bundle exec ruby test/integration_async_test.rb --verbose)

@dentarg
Copy link
Member Author

dentarg commented Feb 12, 2023 •
edited
Loading

No problem when running only async tests (bundle exec ruby test/integration_async_test.rb --verbose)

Or only test/integration_test.rb

Running TESTOPTS="--name='/starts_the_correct_server/'" bundle exec rake triggers it (besides the full run)

@dentarg
Copy link
Member Author

dentarg commented Feb 12, 2023

So the regular integration tests starts all servers before running the tests:

sinatra/test/integration_helper.rb

Lines 128 to 143 in f10e571

def self.extend_object(obj)
super
base_port = 5000 + Process.pid % 100
servers = Sinatra::Base.server.dup
# TruffleRuby doesn't support `Fiber.set_scheduler` yet
if RUBY_ENGINE == "truffleruby" && !Fiber.respond_to?(:set_scheduler)
warn "skip falcon server"
servers.delete('falcon')
end
servers.each_with_index do |server, index|
Server.run(server, base_port+index)
end
end

but for the async tests, the servers are started much later, during run_test, the run call in

sinatra/test/integration_helper.rb

Lines 107 to 117 in f10e571

def run_test(target, &block)
retries ||= 3
target.server = self
run unless alive?
target.instance_eval(&block)
rescue Exception => error
retries -= 1
kill
retries < 0 ? retry : raise(error)
end
end

dentarg added a commit that referenced this issue Feb 12, 2023
@dentarg
Start servers for async tests before running tests
60f11ae 
Close #1874
dentarg added a commit that referenced this issue Feb 12, 2023
@dentarg
Start servers for async tests before running tests
ffb15d5 
Close #1874
@dentarg dentarg mentioned this issue Feb 12, 2023
Merged
dentarg added a commit that referenced this issue Feb 12, 2023
@dentarg
Start servers for async tests before running tests (#1876)
e19cad1 
Close #1874
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dentarg dentarg

Labels
ci / testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Start servers for async tests before running tests sinatra/sinatra
1 participant
@dentarg