From 75c5d6213e96ebe4c24e0788b3b730dfccc1c9d7 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 20 Aug 2024 15:38:36 +0200 Subject: [PATCH 1/2] Migrate to megatest - Change the base class. - Remove requires. - Add some dummy asserts for tests without assertions. --- Gemfile | 2 +- Rakefile | 39 ++++++++++++--------------------- bin/megatest | 27 +++++++++++++++++++++++ test/integration/test_boot.rb | 6 ++--- test/integration_test_helper.rb | 4 ++-- test/slow/test_server.rb | 4 ++++ test/slow/test_signals.rb | 2 ++ test/slow/test_upload.rb | 2 ++ test/test_helper.rb | 3 +-- test/unit/test_configurator.rb | 6 ++--- test/unit/test_http_parser.rb | 18 +++++++-------- test/unit/test_socket_helper.rb | 15 ++++++++----- 12 files changed, 78 insertions(+), 50 deletions(-) create mode 100755 bin/megatest diff --git a/Gemfile b/Gemfile index df9ecfba..8ffbab8c 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem 'minitest' +gem 'megatest', '>= 0.1.1' gem 'rake' gem 'rake-compiler' if ENV["RACK_VERSION"] diff --git a/Rakefile b/Rakefile index 58489004..ed81ae96 100644 --- a/Rakefile +++ b/Rakefile @@ -1,39 +1,28 @@ # frozen_string_literal: true require "bundler/gem_tasks" -require "rake/testtask" require "rake/extensiontask" -Rake::TestTask.new("test:unit") do |t| - t.libs << "test" - t.libs << "lib" - t.test_files = FileList["test/unit/**/test_*.rb"] - t.options = '-v' if ENV['CI'] || ENV['VERBOSE'] - t.warning = true -end - -Rake::TestTask.new("test:integration") do |t| - t.libs << "test" - t.libs << "lib" - t.test_files = FileList["test/integration/**/test_*.rb"] - t.options = '-v' if ENV['CI'] || ENV['VERBOSE'] - t.warning = true -end +require "megatest/test_task" namespace :test do + Megatest::TestTask.create(:unit) do |t| + t.tests = FileList["test/unit/**/test_*.rb"] + t.deps << :compile + end + + Megatest::TestTask.create(:integration) do |t| + t.tests = FileList["test/integration/**/test_*.rb"] + t.deps << :compile + end + # It's not so much that these tests are slow, but they tend to fork # and/or register signal handlers, so they if something goes wrong # they are likely to get stuck forever. # The unicorn test suite has historically ran them in individual process # so we continue to do that. - task slow: :compile do - tests = Dir["test/slow/**/*.rb"].flat_map do |test_file| - File.read(test_file).scan(/def (test_\w+)/).map do |test| - [test_file] + test - end - end - tests.each do |file, test| - sh "ruby", "-Ilib:test", file, "-n", test, "-v" - end + Megatest::TestTask.create(:slow) do |t| + t.tests = FileList["test/slow/**/test_*.rb"] + t.deps << :compile end # Unicorn had its own POSIX-shell based testing framework. diff --git a/bin/megatest b/bin/megatest new file mode 100755 index 00000000..3538fba5 --- /dev/null +++ b/bin/megatest @@ -0,0 +1,27 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# +# This file was generated by Bundler. +# +# The application 'megatest' is installed as part of a gem, and +# this file is here to facilitate running it. +# + +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) + +bundle_binstub = File.expand_path("bundle", __dir__) + +if File.file?(bundle_binstub) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") + load(bundle_binstub) + else + abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. +Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.") + end +end + +require "rubygems" +require "bundler/setup" + +load Gem.bin_path("megatest", "megatest") diff --git a/test/integration/test_boot.rb b/test/integration/test_boot.rb index 0c507186..fe9ca1aa 100644 --- a/test/integration/test_boot.rb +++ b/test/integration/test_boot.rb @@ -51,10 +51,10 @@ def teardown end Dir.chdir(@_old_pwd) - if passed? - FileUtils.rm_rf(@pwd) - else + if __result__.failure? $stderr.puts("Working directory left at: #{@pwd}") + else + FileUtils.rm_rf(@pwd) end end diff --git a/test/integration_test_helper.rb b/test/integration_test_helper.rb index 668da8a9..795e3e15 100644 --- a/test/integration_test_helper.rb +++ b/test/integration_test_helper.rb @@ -2,7 +2,7 @@ require 'test_helper' module Pitchfork - class IntegrationTest < Minitest::Test + class IntegrationTest < Megatest::Test ROOT = File.expand_path('../', __dir__) BIN = File.join(ROOT, 'exe/pitchfork') def setup @@ -193,7 +193,7 @@ def spawn(*args) def print_stderr_on_error yield - rescue Minitest::Assertion + rescue Megatest::Assertion puts '=' * 40 puts read_stderr puts '=' * 40 diff --git a/test/slow/test_server.rb b/test/slow/test_server.rb index 893adda3..f9f73ae9 100644 --- a/test/slow/test_server.rb +++ b/test/slow/test_server.rb @@ -91,6 +91,8 @@ def teardown class WebServerStartTest < Pitchfork::Test include BaseWebServerTests + tag isolated: true + def test_preload_app tmp = Tempfile.new('test_preload_app_config') Pitchfork::Info.keep_io(tmp) @@ -287,6 +289,7 @@ def do_test(string, chunk, close_after=nil, shutdown_delay=0) sleep(shutdown_delay) socket.write(" ") # Some platforms only raise the exception on attempted write socket.flush + assert true end def test_trickle_attack @@ -303,6 +306,7 @@ def test_bad_client redirect_test_io do do_test("GET /test HTTP/BAD", 3) end + assert true end def test_logger_set diff --git a/test/slow/test_signals.rb b/test/slow/test_signals.rb index 3386ef29..b27a1222 100644 --- a/test/slow/test_signals.rb +++ b/test/slow/test_signals.rb @@ -9,6 +9,8 @@ require 'test_helper' class SignalsTest < Pitchfork::Test + tag isolated: true + include Pitchfork class Dd diff --git a/test/slow/test_upload.rb b/test/slow/test_upload.rb index 9766395f..55fcba2d 100644 --- a/test/slow/test_upload.rb +++ b/test/slow/test_upload.rb @@ -5,6 +5,8 @@ require 'test_helper' class UploadTest < Pitchfork::Test + tag isolated: true + include Pitchfork def setup diff --git a/test/test_helper.rb b/test/test_helper.rb index 96a8aead..16e51663 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -19,7 +19,6 @@ DEFAULT_TRIES = 100 DEFAULT_RES = 0.2 -require 'minitest/autorun' require 'net/http' require 'digest/sha1' require 'uri' @@ -38,7 +37,7 @@ end module Pitchfork - class Test < Minitest::Test + class Test < Megatest::Test def before_setup Pitchfork::SharedMemory::PAGES.clear Pitchfork::SharedMemory.preallocate_pages(4) diff --git a/test/unit/test_configurator.rb b/test/unit/test_configurator.rb index 8e9cc958..a3551cf5 100644 --- a/test/unit/test_configurator.rb +++ b/test/unit/test_configurator.rb @@ -7,7 +7,7 @@ class TestConfigurator < Pitchfork::Test *(Pitchfork::Configurator::DEFAULTS.keys + %w(listener_opts listeners))) def test_config_init - Pitchfork::Configurator.new {} + assert(Pitchfork::Configurator.new {}) end def test_expand_addr @@ -127,7 +127,7 @@ def test_listen_option_float_delay expect = { :delay => 0.5 } listener = "127.0.0.1:12345" tmp.syswrite("listen '#{listener}', #{expect.inspect}\n") - Pitchfork::Configurator.new(:config_file => tmp.path) + assert Pitchfork::Configurator.new(:config_file => tmp.path) end def test_listen_option_int_delay @@ -135,7 +135,7 @@ def test_listen_option_int_delay expect = { :delay => 5 } listener = "127.0.0.1:12345" tmp.syswrite("listen '#{listener}', #{expect.inspect}\n") - Pitchfork::Configurator.new(:config_file => tmp.path) + assert Pitchfork::Configurator.new(:config_file => tmp.path) end def test_check_client_connection diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb index a6e9eb3b..a752bbf2 100644 --- a/test/unit/test_http_parser.rb +++ b/test/unit/test_http_parser.rb @@ -873,15 +873,15 @@ def test_non_ascii_header_encoding end def test_memsize - if ObjectSpace.respond_to?(:memsize_of) - n = ObjectSpace.memsize_of(Pitchfork::HttpParser.new) - assert_kind_of Integer, n - # need to update this when 128-bit machines come out - # n.b. actual struct size on 64-bit is 56 bytes + 40 bytes for RVALUE - # Ruby <= 2.2 objspace did not count the 40-byte RVALUE, 2.3 does. - assert_operator n, :<=, 96 - assert_operator n, :>, 0 - end + require "objspace" + + n = ObjectSpace.memsize_of(Pitchfork::HttpParser.new) + assert_kind_of Integer, n + # need to update this when 128-bit machines come out + # n.b. actual struct size on 64-bit is 56 bytes + 40 bytes for RVALUE + # Ruby <= 2.2 objspace did not count the 40-byte RVALUE, 2.3 does. + assert_operator n, :<=, 96 + assert_operator n, :>, 0 end def test_dedupe diff --git a/test/unit/test_socket_helper.rb b/test/unit/test_socket_helper.rb index 784eddcd..7889abbe 100644 --- a/test/unit/test_socket_helper.rb +++ b/test/unit/test_socket_helper.rb @@ -150,7 +150,8 @@ def test_sock_name end def test_tcp_defer_accept_default - return unless defined?(TCP_DEFER_ACCEPT) + skip("Missing TCP_DEFER_ACCEPT") unless defined?(TCP_DEFER_ACCEPT) + port = unused_port @test_addr name = "#@test_addr:#{port}" sock = bind_listen(name) @@ -159,7 +160,8 @@ def test_tcp_defer_accept_default end def test_tcp_defer_accept_disable - return unless defined?(TCP_DEFER_ACCEPT) + skip("Missing TCP_DEFER_ACCEPT") unless defined?(TCP_DEFER_ACCEPT) + port = unused_port @test_addr name = "#@test_addr:#{port}" sock = bind_listen(name, :tcp_defer_accept => false) @@ -168,7 +170,8 @@ def test_tcp_defer_accept_disable end def test_tcp_defer_accept_nr - return unless defined?(TCP_DEFER_ACCEPT) + skip("Missing TCP_DEFER_ACCEPT") unless defined?(TCP_DEFER_ACCEPT) + port = unused_port @test_addr name = "#@test_addr:#{port}" sock = bind_listen(name, :tcp_defer_accept => 60) @@ -180,16 +183,18 @@ def test_ipv6only port = begin unused_port "#@test6_addr" rescue Errno::EINVAL - return + skip("IPv6: EINVAL") end sock = bind_listen "[#@test6_addr]:#{port}", :ipv6only => true cur = sock.getsockopt(:IPPROTO_IPV6, :IPV6_V6ONLY).unpack('i')[0] assert_equal 1, cur rescue Errno::EAFNOSUPPORT + skip("IPv6: EAFNOSUPPORT") end def test_reuseport - return unless defined?(Socket::SO_REUSEPORT) + skip("Missing Socket::SO_REUSEPORT") unless defined?(TCP_DEFER_ACCEPT) + port = unused_port @test_addr name = "#@test_addr:#{port}" sock = bind_listen(name, :reuseport => true) From bbc4ab19aef537b7f629715ac3214243b1d694f2 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 20 Aug 2024 16:51:31 +0200 Subject: [PATCH 2/2] Reimplement SignalsTest#test_worker_dies_on_dead_master --- test/integration/test_boot.rb | 34 ++++++++++++++++++++++++++++++++- test/integration_test_helper.rb | 6 +++--- test/slow/test_signals.rb | 31 ------------------------------ 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/test/integration/test_boot.rb b/test/integration/test_boot.rb index fe9ca1aa..d53e9b90 100644 --- a/test/integration/test_boot.rb +++ b/test/integration/test_boot.rb @@ -105,9 +105,41 @@ def test_boot_worker_stuck_in_spawn assert_healthy("http://#{addr}:#{port}") assert_stderr("worker=0 gen=0 ready") - assert_stderr(/worker=1 pid=\d+ gen=0 registered/) + assert_stderr(/worker=1 pid=(\d+) gen=0 registered/) assert_stderr(/worker=1 pid=\d+ gen=0 timed out, killing/, timeout: 4) assert_clean_shutdown(pid) end + + test "workers and mold exit on monitor crash", isolated: true do + skip("Missing CHILD_SUBREAPER") unless Pitchfork::CHILD_SUBREAPER_AVAILABLE + + Pitchfork.enable_child_subreaper + + addr, port = unused_port + + pid = spawn_server(app: APP, config: <<~RUBY) + listen "#{addr}:#{port}" + worker_processes 2 + timeout 3 + refork_after [50, 100, 1000] + RUBY + + assert_healthy("http://#{addr}:#{port}") + assert_stderr("worker=0 gen=0 ready") + assert_stderr("worker=1 gen=0 ready") + + Process.kill(:KILL, pid) + Process.waitpid(pid) + + assert_stderr(/worker=0 pid=(\d+) gen=0 exiting/, timeout: 5) + assert_stderr(/worker=1 pid=(\d+) gen=0 exiting/) + + assert_raises Errno::ESRCH, Errno::ECHILD do + 25.times do + Process.wait(-1, Process::WNOHANG) + sleep 0.2 + end + end + end end diff --git a/test/integration_test_helper.rb b/test/integration_test_helper.rb index 795e3e15..6176cf0f 100644 --- a/test/integration_test_helper.rb +++ b/test/integration_test_helper.rb @@ -50,14 +50,14 @@ def teardown end Dir.chdir(@_old_pwd) - if passed? || skipped? - FileUtils.rm_rf(@pwd) - else + if __result__.failure? $stderr.puts("Working directory left at: #{@pwd}") if ENV["CI"] $stderr.puts "-" * 40 $stderr.puts(File.read(File.join(@pwd, "stderr.log"))) end + else + FileUtils.rm_rf(@pwd) end super diff --git a/test/slow/test_signals.rb b/test/slow/test_signals.rb index b27a1222..37137ee2 100644 --- a/test/slow/test_signals.rb +++ b/test/slow/test_signals.rb @@ -46,37 +46,6 @@ def teardown reset_sig_handlers end - def test_worker_dies_on_dead_master - pid = fork { - app = lambda { |env| [ 200, {'X-Pid' => "#$$" }, [] ] } - opts = @server_opts.merge(:timeout => 3) - redirect_test_io { HttpServer.new(app, opts).start.join } - } - wait_workers_ready("test_stderr.#{pid}.log", 1) - sock = tcp_socket('127.0.0.1', @port) - sock.syswrite("GET / HTTP/1.0\r\n\r\n") - buf = sock.readpartial(4096) - assert_nil sock.close - buf =~ /\bX-Pid: (\d+)\b/ or raise Exception - child = $1.to_i - wait_master_ready("test_stderr.#{pid}.log") - wait_workers_ready("test_stderr.#{pid}.log", 1) - Process.kill(:KILL, pid) - Process.waitpid(pid) - File.unlink("test_stderr.#{pid}.log", "test_stdout.#{pid}.log") - t0 = Time.now - assert child - assert t0 - assert_raises(Errno::ESRCH) { 25.times { Process.kill(0, child); sleep 0.2 } } - child = nil - assert((Time.now - t0) < 60) - ensure - if child - puts "SIGKILL child pid=#{child}" - Process.kill(:KILL, child) - end - end - def test_sleepy_kill rd, wr = Pitchfork::Info.keep_ios(IO.pipe) pid = fork {