From fa65cf714101838c13847045a66a8d197803669e Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Fri, 30 Sep 2022 01:06:32 -0500 Subject: [PATCH] 103 RuboCop fixes (#2976) * rubocop.yml - add more Cops & alphabetize * RuboCop - Performance/UnfreezeString * RuboCop - Style/SafeNavigation * RuboCop - Performance/StringInclude * RuboCop - Performance/StringIdentifierArgument * RuboCop - Performance/RegexpMatch * RuboCop - Performance/MethodObjectAsBlock * RuboCop - Performance/CollectionLiteralInLoop * RuboCop - Performance/ChainArrayAllocation --- .rubocop.yml | 60 +++++++++++++++--------- benchmarks/local/bench_base.rb | 6 ++- benchmarks/local/response_time_wrk.rb | 2 +- bin/puma-wild | 2 +- lib/puma/binder.rb | 17 ++++--- lib/puma/client.rb | 4 +- lib/puma/cluster.rb | 10 ++-- lib/puma/dsl.rb | 2 +- lib/puma/jruby_restart.rb | 3 +- lib/puma/launcher/bundle_pruner.rb | 5 +- lib/puma/minissl.rb | 2 +- lib/puma/rack/builder.rb | 6 +-- lib/puma/reactor.rb | 4 +- lib/puma/request.rb | 4 +- lib/puma/runner.rb | 2 +- lib/puma/server.rb | 10 ++-- lib/puma/single.rb | 10 ++-- lib/puma/state_file.rb | 2 +- lib/puma/thread_pool.rb | 6 +-- test/helper.rb | 2 +- test/helpers/integration.rb | 32 ++++++------- test/minitest/verbose_progress_plugin.rb | 2 +- test/rackup/ci_array.ru | 4 +- test/rackup/ci_chunked.ru | 4 +- test/rackup/ci_select.ru | 8 ++-- test/rackup/ci_string.ru | 8 ++-- test/rackup/env-dump.ru | 2 +- test/test_busy_worker.rb | 2 +- test/test_cli.rb | 5 +- test/test_config.rb | 2 +- test/test_integration_cluster.rb | 6 ++- test/test_integration_ssl_session.rb | 2 +- test/test_integration_systemd.rb | 2 +- test/test_out_of_band_server.rb | 2 +- test/test_persistent.rb | 2 +- test/test_puma_localhost_authority.rb | 4 +- test/test_puma_server_ssl.rb | 8 ++-- test/test_pumactl.rb | 40 ++++++++-------- test/test_rack_handler.rb | 4 +- test/test_rack_server.rb | 18 ++++--- test/test_request_invalid.rb | 2 +- test/test_worker_gem_independence.rb | 2 +- 42 files changed, 171 insertions(+), 149 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6f8779c8..7e2b31f7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,17 +16,14 @@ AllCops: # ————————————————————————————————————————— disabled cops -Performance/RegexpMatch: - Enabled: false - -Performance/UnfreezeString: - Enabled: false - -Style/RedundantReturn: - Enabled: false - # ————————————————————————————————————————— enabled cops +Layout/AccessModifierIndentation: + EnforcedStyle: indent + +Layout/IndentationStyle: + Enabled: true + Layout/SpaceAfterColon: Enabled: true @@ -43,9 +40,6 @@ Layout/SpaceBeforeFirstArg: Layout/SpaceInsideParens: Enabled: true -Layout/IndentationStyle: - Enabled: true - Layout/TrailingEmptyLines: Enabled: true @@ -55,6 +49,9 @@ Layout/TrailingWhitespace: Lint/Debugger: Enabled: true +Metrics/ParameterLists: + Max: 7 + Naming/MethodName: Enabled: true EnforcedStyle: snake_case @@ -64,17 +61,23 @@ Naming/MethodName: Naming/VariableName: Enabled: true -Style/MethodDefParentheses: - Enabled: true - -Style/TrailingCommaInArguments: - Enabled: true - Performance: Enabled: true -Metrics/ParameterLists: - Max: 7 +Performance/ChainArrayAllocation: + Enabled: true + +Performance/CollectionLiteralInLoop: + Enabled: true + +Performance/DeletePrefix: + Enabled: true + +Performance/DeleteSuffix: + Enabled: true + +Performance/MethodObjectAsBlock: + Enabled: true Performance/RedundantMatch: Enabled: true @@ -82,15 +85,26 @@ Performance/RedundantMatch: Performance/RedundantBlockCall: Enabled: true +Performance/StringIdentifierArgument: + Enabled: true + +Performance/StringInclude: + Enabled: true + Performance/StringReplacement: Enabled: true -Layout/AccessModifierIndentation: - EnforcedStyle: indent +Style/MethodDefParentheses: + Enabled: true -Style/WhileUntilModifier: +Style/SafeNavigation: Enabled: true Style/TernaryParentheses: Enabled: true +Style/TrailingCommaInArguments: + Enabled: true + +Style/WhileUntilModifier: + Enabled: true diff --git a/benchmarks/local/bench_base.rb b/benchmarks/local/bench_base.rb index 7c271431..3ecad884 100644 --- a/benchmarks/local/bench_base.rb +++ b/benchmarks/local/bench_base.rb @@ -124,7 +124,9 @@ module TestPuma end if (sizes = arg[SIZES_RE]) - @body_sizes = sizes.split(',').map(&:to_i).sort + @body_sizes = sizes.split(',') + @body_sizes.map!(&:to_i) + @body_sizes.sort! end end @@ -366,7 +368,7 @@ module TestPuma # @param summaries [Hash] generated in subclasses # def overall_summary(summaries) - names = ''.dup + names = +'' @body_types.each { |_, t_desc| names << t_desc.rjust(8) } puts "\nBody ────────── req/sec ────────── ─────── req 50% times ───────" \ diff --git a/benchmarks/local/response_time_wrk.rb b/benchmarks/local/response_time_wrk.rb index b297fbc8..cbc437fc 100644 --- a/benchmarks/local/response_time_wrk.rb +++ b/benchmarks/local/response_time_wrk.rb @@ -113,7 +113,7 @@ module TestPuma def run_summaries(summaries) digits = [4 - Math.log10(@max_100_time).to_i, 3].min - fmt_vals = "%-6s %6d".dup + fmt_vals = +'%-6s %6d' fmt_vals << (digits < 0 ? " %6d" : " %6.#{digits}f")*5 fmt_vals << ' %8d' diff --git a/bin/puma-wild b/bin/puma-wild index 5db409f7..3701b210 100644 --- a/bin/puma-wild +++ b/bin/puma-wild @@ -16,7 +16,7 @@ end module Puma; end -Puma.const_set("WILD_ARGS", ["-I", inc]) +Puma.const_set(:WILD_ARGS, ["-I", inc]) require 'puma/cli' diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index a75289a1..86d7df26 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -70,7 +70,7 @@ module Puma # @!attribute [r] connected_ports # @version 5.0.0 def connected_ports - ios.map { |io| io.addr[1] }.uniq + t = ios.map { |io| io.addr[1] }; t.uniq!; t end # @version 5.0.0 @@ -96,7 +96,7 @@ module Puma [:unix, Socket.unpack_sockaddr_un(sock.getsockname)] rescue ArgumentError # Try to parse as a port/ip port, addr = Socket.unpack_sockaddr_in(sock.getsockname) - addr = "[#{addr}]" if addr =~ /\:/ + addr = "[#{addr}]" if addr&.include? ':' [:tcp, addr, port] end @activated_sockets[key] = sock @@ -216,6 +216,7 @@ module Puma @listeners << [str, io] when "ssl" + cert_key = %w[cert key] raise "Puma compiled without SSL support" unless HAS_SSL @@ -224,15 +225,16 @@ module Puma # If key and certs are not defined and localhost gem is required. # localhost gem will be used for self signed # Load localhost authority if not loaded. - if params.values_at('cert', 'key').all? { |v| v.to_s.empty? } + # Ruby 3 `values_at` accepts an array, earlier do not + if params.values_at(*cert_key).all? { |v| v.to_s.empty? } ctx = localhost_authority && localhost_authority_context end ctx ||= begin # Extract cert_pem and key_pem from options[:store] if present - ['cert', 'key'].each do |v| - if params[v] && params[v].start_with?('store:') + cert_key.each do |v| + if params[v]&.start_with?('store:') index = Integer(params.delete(v).split('store:').last) params["#{v}_pem"] = @conf.options[:store][index] end @@ -473,9 +475,10 @@ module Puma # @!attribute [r] loopback_addresses def loopback_addresses - Socket.ip_address_list.select do |addrinfo| + t = Socket.ip_address_list.select do |addrinfo| addrinfo.ipv6_loopback? || addrinfo.ipv4_loopback? - end.map { |addrinfo| addrinfo.ip_address }.uniq + end + t.map! { |addrinfo| addrinfo.ip_address }; t.uniq!; t end def loc_addr_str(io) diff --git a/lib/puma/client.rb b/lib/puma/client.rb index 121c82c6..96799ad8 100644 --- a/lib/puma/client.rb +++ b/lib/puma/client.rb @@ -355,7 +355,7 @@ module Puma if cl # cannot contain characters that are not \d - if cl =~ CONTENT_LENGTH_VALUE_INVALID + if CONTENT_LENGTH_VALUE_INVALID.match? cl raise HttpParserError, "Invalid Content-Length: #{cl.inspect}" end else @@ -520,7 +520,7 @@ module Puma # Puma doesn't process chunk extensions, but should parse if they're # present, which is the reason for the semicolon regex chunk_hex = line.strip[/\A[^;]+/] - if chunk_hex =~ CHUNK_SIZE_INVALID + if CHUNK_SIZE_INVALID.match? chunk_hex raise HttpParserError, "Invalid chunk size: '#{chunk_hex}'" end len = chunk_hex.to_i(16) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 07011f74..d3374633 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -180,10 +180,10 @@ module Puma end end - @next_check = [ - @workers.reject(&:term?).map(&:ping_timeout).min, - @next_check - ].compact.min + t = @workers.reject(&:term?) + t.map!(&:ping_timeout) + + @next_check = [t.min, @next_check].compact.min end def worker(index, master) @@ -230,7 +230,7 @@ module Puma def stop_blocked @status = :stop if @status == :run wakeup! - @control.stop(true) if @control + @control&.stop true Process.waitall end diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb index 1b48cbbe..d1b4ec62 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -94,7 +94,7 @@ module Puma if reuse == true '&reuse=dflt' elsif reuse.is_a?(Hash) && (reuse.key?(:size) || reuse.key?(:timeout)) - val = ''.dup + val = +'' if (size = reuse[:size]) && Integer === size val << size.to_s end diff --git a/lib/puma/jruby_restart.rb b/lib/puma/jruby_restart.rb index af16d5b1..5690c4e3 100644 --- a/lib/puma/jruby_restart.rb +++ b/lib/puma/jruby_restart.rb @@ -16,7 +16,8 @@ module Puma def self.chdir_exec(dir, argv) chdir(dir) cmd = argv.first - argv = ([:string] * argv.size).zip(argv).flatten + argv = ([:string] * argv.size).zip(argv) + argv.flatten! argv << :string argv << nil execlp(cmd, *argv) diff --git a/lib/puma/launcher/bundle_pruner.rb b/lib/puma/launcher/bundle_pruner.rb index a33858e9..1b5c9116 100644 --- a/lib/puma/launcher/bundle_pruner.rb +++ b/lib/puma/launcher/bundle_pruner.rb @@ -73,14 +73,15 @@ module Puma end def extra_runtime_deps_paths - @extra_runtime_dependencies.map do |dep_name| + t = @extra_runtime_dependencies.map do |dep_name| if (spec = spec_for_gem(dep_name)) require_paths_for_gem(spec) else log "* Could not load extra dependency: #{dep_name}" nil end - end.flatten.compact + end + t.flatten!; t.compact!; t end def puma_require_paths diff --git a/lib/puma/minissl.rb b/lib/puma/minissl.rb index 7f219933..1ac1fe0b 100644 --- a/lib/puma/minissl.rb +++ b/lib/puma/minissl.rb @@ -125,7 +125,7 @@ module Puma while true wrote = @engine.write data - enc_wr = ''.dup + enc_wr = +'' while (enc = @engine.extract) enc_wr << enc end diff --git a/lib/puma/rack/builder.rb b/lib/puma/rack/builder.rb index 9693a6e9..0823d5ac 100644 --- a/lib/puma/rack/builder.rb +++ b/lib/puma/rack/builder.rb @@ -102,13 +102,13 @@ module Puma::Rack begin info = [] server = Rack::Handler.get(options[:server]) || Rack::Handler.default(options) - if server && server.respond_to?(:valid_options) + if server&.respond_to?(:valid_options) info << "" info << "Server-specific options for #{server.name}:" has_options = false server.valid_options.each do |name, description| - next if name.to_s =~ /^(Host|Port)[^a-zA-Z]/ # ignore handler's host and port options, we do our own. + next if /^(Host|Port)[^a-zA-Z]/.match? name.to_s # ignore handler's host and port options, we do our own. info << " -O %-21s %s" % [name, description] has_options = true @@ -276,7 +276,7 @@ module Puma::Rack app = @map ? generate_map(@run, @map) : @run fail "missing run or map statement" unless app app = @use.reverse.inject(app) { |a,e| e[a] } - @warmup.call(app) if @warmup + @warmup&.call app app end diff --git a/lib/puma/reactor.rb b/lib/puma/reactor.rb index f75b6ffe..f86e358d 100644 --- a/lib/puma/reactor.rb +++ b/lib/puma/reactor.rb @@ -61,7 +61,7 @@ module Puma @selector.wakeup rescue IOError # Ignore if selector is already closed end - @thread.join if @thread + @thread&.join end private @@ -76,7 +76,7 @@ module Puma # Wakeup all objects that timed out. timed_out = @timeouts.take_while {|t| t.timeout == 0} - timed_out.each(&method(:wakeup!)) + timed_out.each { |c| wakeup! c } unless @input.empty? until @input.empty? diff --git a/lib/puma/request.rb b/lib/puma/request.rb index c6c2a8bb..7605d087 100644 --- a/lib/puma/request.rb +++ b/lib/puma/request.rb @@ -213,7 +213,7 @@ module Puma resp_info = nil uncork_socket socket app_body.close if app_body.respond_to? :close - client.tempfile.unlink if client.tempfile + client.tempfile&.unlink begin after_reply.each { |o| o.call } @@ -462,7 +462,7 @@ module Puma # @version 5.0.3 # def str_early_hints(headers) - eh_str = "HTTP/1.1 103 Early Hints\r\n".dup + eh_str = +"HTTP/1.1 103 Early Hints\r\n" headers.each_pair do |k, vs| next if illegal_header_key?(k) diff --git a/lib/puma/runner.rb b/lib/puma/runner.rb index 9554390c..20bd26c6 100644 --- a/lib/puma/runner.rb +++ b/lib/puma/runner.rb @@ -47,7 +47,7 @@ module Puma # @version 5.0.0 def stop_control - @control.stop(true) if @control + @control&.stop true end def error(str) diff --git a/lib/puma/server.rb b/lib/puma/server.rb index 77b88ad7..d9e12e81 100644 --- a/lib/puma/server.rb +++ b/lib/puma/server.rb @@ -196,12 +196,12 @@ module Puma # @!attribute [r] backlog def backlog - @thread_pool and @thread_pool.backlog + @thread_pool&.backlog end # @!attribute [r] running def running - @thread_pool and @thread_pool.spawned + @thread_pool&.spawned end @@ -214,7 +214,7 @@ module Puma # value would be 4 until it finishes processing. # @!attribute [r] pool_capacity def pool_capacity - @thread_pool and @thread_pool.pool_capacity + @thread_pool&.pool_capacity end # Runs the server. @@ -230,10 +230,10 @@ module Puma @status = :run - @thread_pool = ThreadPool.new(thread_name, @options, &method(:process_client)) + @thread_pool = ThreadPool.new(thread_name, @options) { |a, b| process_client a, b } if @queue_requests - @reactor = Reactor.new(@io_selector_backend, &method(:reactor_wakeup)) + @reactor = Reactor.new(@io_selector_backend) { |c| reactor_wakeup c } @reactor.run end diff --git a/lib/puma/single.rb b/lib/puma/single.rb index 14a5b122..447b4d85 100644 --- a/lib/puma/single.rb +++ b/lib/puma/single.rb @@ -21,21 +21,21 @@ module Puma end def restart - @server.begin_restart + @server&.begin_restart end def stop - @server.stop(false) if @server + @server&.stop false end def halt - @server.halt + @server&.halt end def stop_blocked log "- Gracefully stopping, waiting for requests to finish" - @control.stop(true) if @control - @server.stop(true) if @server + @control&.stop true + @server&.stop true end def run diff --git a/lib/puma/state_file.rb b/lib/puma/state_file.rb index b55f37bb..f030b627 100644 --- a/lib/puma/state_file.rb +++ b/lib/puma/state_file.rb @@ -20,7 +20,7 @@ module Puma end def save(path, permission = nil) - contents = "---\n".dup + contents = +"---\n" @options.each do |k,v| next unless ALLOWED_FIELDS.include? k case v diff --git a/lib/puma/thread_pool.rb b/lib/puma/thread_pool.rb index 482ecedf..dd622bd9 100644 --- a/lib/puma/thread_pool.rb +++ b/lib/puma/thread_pool.rb @@ -163,7 +163,7 @@ module Puma # @version 5.0.0 def trigger_out_of_band_hook - return false unless @out_of_band && @out_of_band.any? + return false unless @out_of_band&.any? # we execute on idle hook when all threads are free return false unless @spawned == @waiting @@ -357,8 +357,8 @@ module Puma @not_empty.broadcast @not_full.broadcast - @auto_trim.stop if @auto_trim - @reaper.stop if @reaper + @auto_trim&.stop + @reaper&.stop # dup workers so that we join them all safely @workers.dup end diff --git a/test/helper.rb b/test/helper.rb index 0ea06ade..f59656eb 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -21,7 +21,7 @@ require_relative "helpers/apps" Thread.abort_on_exception = true -$debugging_info = ''.dup +$debugging_info = +'' $debugging_hold = false # needed for TestCLI#test_control_clustered $test_case_timeout = ENV.fetch("TEST_CASE_TIMEOUT") do RUBY_ENGINE == "ruby" ? 45 : 60 diff --git a/test/helpers/integration.rb b/test/helpers/integration.rb index 64234af0..54d6af2b 100644 --- a/test/helpers/integration.rb +++ b/test/helpers/integration.rb @@ -34,14 +34,12 @@ class TestIntegration < Minitest::Test stop_server @pid, signal: :INT end - if @ios_to_close - @ios_to_close.each do |io| - begin - io.close if io.respond_to?(:close) && !io.closed? - rescue - ensure - io = nil - end + @ios_to_close&.each do |io| + begin + io.close if io.respond_to?(:close) && !io.closed? + rescue + ensure + io = nil end end @@ -136,9 +134,9 @@ class TestIntegration < Minitest::Test if log puts "Waiting for '#{str}'" begin - line = @server && @server.gets - puts line if line && !line.strip.empty? - end until line && line.include?(str) + line = @server&.gets + puts line if !line&.strip.empty? + end until line&.include?(str) else true until (@server.gets || '').include?(str) end @@ -163,9 +161,9 @@ class TestIntegration < Minitest::Test if log puts "Waiting for '#{re.inspect}'" begin - line = @server && @server.gets - puts line if line && !line.strip.empty? - end until line && line.match?(re) + line = @server&.gets + puts line if !line&.strip.empty? + end until line&.match?(re) else true until (line = @server.gets || '').match?(re) end @@ -222,7 +220,7 @@ class TestIntegration < Minitest::Test timeout ||= RESP_READ_TIMEOUT content_length = nil chunked = nil - response = ''.dup + response = +'' t_st = Process.clock_gettime Process::CLOCK_MONOTONIC if connection.to_io.wait_readable timeout loop do @@ -399,8 +397,10 @@ class TestIntegration < Minitest::Test if Puma.windows? cli_pumactl 'stop' Process.wait @server.pid - @server = nil + else + stop_server end + @server = nil msg = (" %4d unexpected_response\n" % replies.fetch(:unexpected_response,0)).dup msg << " %4d refused\n" % replies.fetch(:refused,0) diff --git a/test/minitest/verbose_progress_plugin.rb b/test/minitest/verbose_progress_plugin.rb index 4c905e68..372511e7 100644 --- a/test/minitest/verbose_progress_plugin.rb +++ b/test/minitest/verbose_progress_plugin.rb @@ -12,7 +12,7 @@ module Minitest class VerboseProgressReporter < Reporter def prerecord(klass, name) @current ||= nil - @current = [klass.name, name].tap(&method(:print_start)) + @current = [klass.name, name].tap { |t| print_start t } end def record(result) diff --git a/test/rackup/ci_array.ru b/test/rackup/ci_array.ru index f3309342..36e93ded 100644 --- a/test/rackup/ci_array.ru +++ b/test/rackup/ci_array.ru @@ -22,11 +22,11 @@ cache_array = {} run lambda { |env| info = if (dly = env[hdr_dly]) - hash_key = "#{dly},".dup + hash_key = +"#{dly}," sleep dly.to_f "#{Process.pid}\nHello World\nSlept #{dly}\n" else - hash_key = ",".dup + hash_key = +"," "#{Process.pid}\nHello World\n" end info_len_adj = 1023 - info.bytesize diff --git a/test/rackup/ci_chunked.ru b/test/rackup/ci_chunked.ru index 0cdfb59d..fb389359 100644 --- a/test/rackup/ci_chunked.ru +++ b/test/rackup/ci_chunked.ru @@ -22,11 +22,11 @@ cache_chunked = {} run lambda { |env| info = if (dly = env[hdr_dly]) - hash_key = "#{dly},".dup + hash_key = +"#{dly}," sleep dly.to_f "#{Process.pid}\nHello World\nSlept #{dly}\n" else - hash_key = ",".dup + hash_key = +"," "#{Process.pid}\nHello World\n" end info_len_adj = 1023 - info.bytesize diff --git a/test/rackup/ci_select.ru b/test/rackup/ci_select.ru index 20f85ab5..3f847f5e 100644 --- a/test/rackup/ci_select.ru +++ b/test/rackup/ci_select.ru @@ -28,12 +28,12 @@ cache_string = {} run lambda { |env| info = if (dly = env[hdr_dly]) - hash_key = "#{dly},".dup + hash_key = +"#{dly}," sleep dly.to_f - "#{Process.pid}\nHello World\nSlept #{dly}\n".dup + +"#{Process.pid}\nHello World\nSlept #{dly}\n" else - hash_key = ",".dup - "#{Process.pid}\nHello World\n".dup + hash_key = +"," + +"#{Process.pid}\nHello World\n" end info_len_adj = 1023 - info.bytesize diff --git a/test/rackup/ci_string.ru b/test/rackup/ci_string.ru index b44f47dc..2195830d 100644 --- a/test/rackup/ci_string.ru +++ b/test/rackup/ci_string.ru @@ -25,12 +25,12 @@ cache_string = {} run lambda { |env| info = if (dly = env[hdr_dly]) - hash_key = "#{dly},".dup + +hash_key = "#{dly}," sleep dly.to_f - "#{Process.pid}\nHello World\nSlept #{dly}\n".dup + +"#{Process.pid}\nHello World\nSlept #{dly}\n" else - hash_key = ",".dup - "#{Process.pid}\nHello World\n".dup + +hash_key = "," + +"#{Process.pid}\nHello World\n" end info_len_adj = 1023 - info.bytesize diff --git a/test/rackup/env-dump.ru b/test/rackup/env-dump.ru index d1652bd9..e7f28da7 100644 --- a/test/rackup/env-dump.ru +++ b/test/rackup/env-dump.ru @@ -1,5 +1,5 @@ run lambda { |env| - body = "#{'─' * 70} Headers\n".dup + body = +"#{'─' * 70} Headers\n" env.sort.each { |k,v| body << "#{k.ljust 30} #{v}\n" } body << "#{'─' * 78}\n" [200, {"Content-Type" => "text/plain"}, [body]] diff --git a/test/test_busy_worker.rb b/test/test_busy_worker.rb index 8af7d9e9..11973a51 100644 --- a/test/test_busy_worker.rb +++ b/test/test_busy_worker.rb @@ -9,7 +9,7 @@ class TestBusyWorker < Minitest::Test def teardown return if skipped? - @server.stop(true) if @server + @server&.stop true @ios.each {|i| i.close unless i.closed?} end diff --git a/test/test_cli.rb b/test/test_cli.rb index 06938442..d824585b 100644 --- a/test/test_cli.rb +++ b/test/test_cli.rb @@ -106,8 +106,9 @@ class TestCLI < Minitest::Test assert_match(expected_stats, body.split(/\r?\n/).last) ensure - cli.launcher.stop if cli - t.join if t + # always called, even if skipped + cli&.launcher&.stop + t&.join end def test_control_clustered diff --git a/test/test_config.rb b/test/test_config.rb index ef7abc00..678b1534 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -55,7 +55,7 @@ class TestConfigFile < TestConfigFileBase app = conf.app assert bind_configuration =~ %r{ca=.*ca.crt} - assert bind_configuration =~ /verify_mode=peer/ + assert bind_configuration&.include?('verify_mode=peer') assert_equal [200, {}, ["embedded app"]], app.call({}) end diff --git a/test/test_integration_cluster.rb b/test/test_integration_cluster.rb index e4c260f0..31c15215 100644 --- a/test/test_integration_cluster.rb +++ b/test/test_integration_cluster.rb @@ -553,7 +553,9 @@ RUBY read_timeouts = replies.count { |r| r == :read_timeout } # get pids from replies, generate uniq array - qty_pids = replies.map { |body| body[/\d+\z/] }.uniq.compact.length + t = replies.map { |body| body[/\d+\z/] } + t.uniq!; t.compact! + qty_pids = t.length msg = "#{responses} responses, #{qty_pids} uniq pids" @@ -630,7 +632,7 @@ RUBY rescue Errno::ESRCH nil end - end.compact + end.compact! end # used in loop to create several 'requests' diff --git a/test/test_integration_ssl_session.rb b/test/test_integration_ssl_session.rb index ffc72d66..8309b7eb 100644 --- a/test/test_integration_ssl_session.rb +++ b/test/test_integration_ssl_session.rb @@ -22,7 +22,7 @@ class TestIntegrationSSLSession < TestIntegration CERT_PATH = File.expand_path "../examples/puma/client-certs", __dir__ def teardown - @server.close unless @server && @server.closed? + @server.close unless @server&.closed? @server = nil super end diff --git a/test/test_integration_systemd.rb b/test/test_integration_systemd.rb index 151a2e07..42d71e78 100644 --- a/test/test_integration_systemd.rb +++ b/test/test_integration_systemd.rb @@ -23,7 +23,7 @@ class TestIntegrationSystemd < TestIntegration def teardown return if skipped? - @socket.close if @socket + @socket&.close File.unlink(@sockaddr) if @sockaddr @socket = nil @sockaddr = nil diff --git a/test/test_out_of_band_server.rb b/test/test_out_of_band_server.rb index aabd3d5a..5f20b02d 100644 --- a/test/test_out_of_band_server.rb +++ b/test/test_out_of_band_server.rb @@ -13,7 +13,7 @@ class TestOutOfBandServer < Minitest::Test def teardown @oob_finished.broadcast @app_finished.broadcast - @server.stop(true) if @server + @server&.stop true @ios.each do |io| begin diff --git a/test/test_persistent.rb b/test/test_persistent.rb index d022eb17..08568ef3 100644 --- a/test/test_persistent.rb +++ b/test/test_persistent.rb @@ -37,7 +37,7 @@ class TestPersistent < Minitest::Test end def lines(count, s=@client) - str = "".dup + str = +'' Timeout.timeout(5) do count.times { str << (s.gets || "") } end diff --git a/test/test_puma_localhost_authority.rb b/test/test_puma_localhost_authority.rb index c509f425..84ca6c79 100644 --- a/test/test_puma_localhost_authority.rb +++ b/test/test_puma_localhost_authority.rb @@ -21,8 +21,8 @@ class TestPumaLocalhostAuthority < Minitest::Test end def teardown - @http.finish if @http && @http.started? - @server.stop(true) if @server + @http.finish if @http&.started? + @server&.stop true end # yields ctx to block, use for ctx setup & configuration diff --git a/test/test_puma_server_ssl.rb b/test/test_puma_server_ssl.rb index 22f89bd0..0da0af9a 100644 --- a/test/test_puma_server_ssl.rb +++ b/test/test_puma_server_ssl.rb @@ -31,8 +31,8 @@ class TestPumaServerSSL < Minitest::Test end def teardown - @http.finish if @http && @http.started? - @server.stop(true) if @server + @http.finish if @http&.started? + @server&.stop true end # yields ctx to block, use for ctx setup & configuration @@ -347,7 +347,7 @@ class TestPumaServerSSLClient < Minitest::Test end assert_equal subject, log_writer.cert.subject.to_s if subject ensure - server.stop(true) if server + server&.stop true end def test_verify_fail_if_no_client_cert @@ -543,6 +543,6 @@ class TestPumaServerSSLWithCertPemAndKeyPem < Minitest::Test assert_nil client_error ensure - server.stop(true) if server + server&.stop true end end if ::Puma::HAS_SSL && !Puma::IS_JRUBY diff --git a/test/test_pumactl.rb b/test/test_pumactl.rb index f1ed5862..fa24cb94 100644 --- a/test/test_pumactl.rb +++ b/test/test_pumactl.rb @@ -14,7 +14,7 @@ class TestPumaControlCli < TestConfigFileBase end def wait_booted - line = @wait.gets until line =~ /Use Ctrl-C to stop/ + line = @wait.gets until line&.include?('Use Ctrl-C to stop') end def teardown @@ -43,71 +43,71 @@ class TestPumaControlCli < TestConfigFileBase def test_config_file control_cli = Puma::ControlCLI.new ["--config-file", "test/config/state_file_testing_config.rb", "halt"] - assert_equal "t3-pid", control_cli.instance_variable_get("@pidfile") + assert_equal "t3-pid", control_cli.instance_variable_get(:@pidfile) File.unlink "t3-pid" if File.file? "t3-pid" end def test_app_env_without_environment with_env('APP_ENV' => 'test') do control_cli = Puma::ControlCLI.new ['halt'] - assert_equal 'test', control_cli.instance_variable_get('@environment') + assert_equal 'test', control_cli.instance_variable_get(:@environment) end end def test_rack_env_without_environment with_env("RACK_ENV" => "test") do control_cli = Puma::ControlCLI.new ["halt"] - assert_equal "test", control_cli.instance_variable_get("@environment") + assert_equal "test", control_cli.instance_variable_get(:@environment) end end def test_app_env_precedence with_env('APP_ENV' => nil, 'RACK_ENV' => nil, 'RAILS_ENV' => 'production') do control_cli = Puma::ControlCLI.new ['halt'] - assert_equal 'production', control_cli.instance_variable_get('@environment') + assert_equal 'production', control_cli.instance_variable_get(:@environment) end with_env('APP_ENV' => nil, 'RACK_ENV' => 'test', 'RAILS_ENV' => 'production') do control_cli = Puma::ControlCLI.new ['halt'] - assert_equal 'test', control_cli.instance_variable_get('@environment') + assert_equal 'test', control_cli.instance_variable_get(:@environment) end with_env('APP_ENV' => 'development', 'RACK_ENV' => 'test', 'RAILS_ENV' => 'production') do control_cli = Puma::ControlCLI.new ['halt'] - assert_equal 'development', control_cli.instance_variable_get('@environment') + assert_equal 'development', control_cli.instance_variable_get(:@environment) control_cli = Puma::ControlCLI.new ['-e', 'test', 'halt'] - assert_equal 'test', control_cli.instance_variable_get('@environment') + assert_equal 'test', control_cli.instance_variable_get(:@environment) end end def test_environment_without_app_env with_env('APP_ENV' => nil, 'RACK_ENV' => nil, 'RAILS_ENV' => nil) do control_cli = Puma::ControlCLI.new ['halt'] - assert_nil control_cli.instance_variable_get('@environment') + assert_nil control_cli.instance_variable_get(:@environment) control_cli = Puma::ControlCLI.new ['-e', 'test', 'halt'] - assert_equal 'test', control_cli.instance_variable_get('@environment') + assert_equal 'test', control_cli.instance_variable_get(:@environment) end end def test_environment_without_rack_env with_env("RACK_ENV" => nil, 'RAILS_ENV' => nil) do control_cli = Puma::ControlCLI.new ["halt"] - assert_nil control_cli.instance_variable_get("@environment") + assert_nil control_cli.instance_variable_get(:@environment) control_cli = Puma::ControlCLI.new ["-e", "test", "halt"] - assert_equal "test", control_cli.instance_variable_get("@environment") + assert_equal "test", control_cli.instance_variable_get(:@environment) end end def test_environment_with_rack_env with_env("RACK_ENV" => "production") do control_cli = Puma::ControlCLI.new ["halt"] - assert_equal "production", control_cli.instance_variable_get("@environment") + assert_equal "production", control_cli.instance_variable_get(:@environment) control_cli = Puma::ControlCLI.new ["-e", "test", "halt"] - assert_equal "test", control_cli.instance_variable_get("@environment") + assert_equal "test", control_cli.instance_variable_get(:@environment) end end @@ -119,12 +119,12 @@ class TestPumaControlCli < TestConfigFileBase with_env("RACK_ENV" => nil) do with_config_file(puma_config_file, port) do control_cli = Puma::ControlCLI.new ["-e", "production", "halt"] - assert_equal puma_config_file, control_cli.instance_variable_get("@config_file") + assert_equal puma_config_file, control_cli.instance_variable_get(:@config_file) end with_config_file(production_config_file, port) do control_cli = Puma::ControlCLI.new ["-e", "production", "halt"] - assert_equal production_config_file, control_cli.instance_variable_get("@config_file") + assert_equal production_config_file, control_cli.instance_variable_get(:@config_file) end end end @@ -137,12 +137,12 @@ class TestPumaControlCli < TestConfigFileBase with_env("RACK_ENV" => nil, 'RAILS_ENV' => nil) do with_config_file(puma_config_file, port) do control_cli = Puma::ControlCLI.new ["halt"] - assert_equal puma_config_file, control_cli.instance_variable_get("@config_file") + assert_equal puma_config_file, control_cli.instance_variable_get(:@config_file) end with_config_file(development_config_file, port) do control_cli = Puma::ControlCLI.new ["halt"] - assert_equal development_config_file, control_cli.instance_variable_get("@config_file") + assert_equal development_config_file, control_cli.instance_variable_get(:@config_file) end end end @@ -154,7 +154,7 @@ class TestPumaControlCli < TestConfigFileBase ] control_cli = Puma::ControlCLI.new opts, @ready, @ready - assert_equal 'none', control_cli.instance_variable_get("@control_auth_token") + assert_equal 'none', control_cli.instance_variable_get(:@control_auth_token) end def test_control_url_and_status @@ -204,7 +204,7 @@ class TestPumaControlCli < TestConfigFileBase "--pid", "1234" ] cmd = Puma::ControlCLI::NO_REQ_COMMANDS.first - log = ''.dup + log = +'' control_cli = Puma::ControlCLI.new (opts + [cmd]), @ready, @ready def control_cli.send_signal diff --git a/test/test_rack_handler.rb b/test/test_rack_handler.rb index 749fc398..11864d72 100644 --- a/test/test_rack_handler.rb +++ b/test/test_rack_handler.rb @@ -43,8 +43,8 @@ if Rack::RELEASE < '3' yield @launcher ensure - @launcher.stop if @launcher - thread.join if thread + @launcher&.stop + thread&.join end def test_handler_boots diff --git a/test/test_rack_server.rb b/test/test_rack_server.rb index b91b89c7..97fe2a9b 100644 --- a/test/test_rack_server.rb +++ b/test/test_rack_server.rb @@ -64,6 +64,12 @@ class TestRackServer < Minitest::Test @server.stop(true) unless @stopped end + def header_hash(socket) + t = socket.readline("\r\n\r\n").split("\r\n") + t.shift; t.map! { |line| line.split(/:\s?/) } + t.to_h + end + def test_lint @checker = ErrorChecker.new ServerLint.new(@simple) @server.app = @checker @@ -135,11 +141,7 @@ class TestRackServer < Minitest::Test socket.puts "Connection: Keep-Alive\r\n" socket.puts "\r\n" - headers = socket.readline("\r\n\r\n") - .split("\r\n") - .drop(1) - .map { |line| line.split(/:\s?/) } - .to_h + headers = header_hash socket content_length = headers["Content-Length"].to_i real_response_body = socket.read(content_length) @@ -198,11 +200,7 @@ class TestRackServer < Minitest::Test socket.puts "Connection: Keep-Alive\r\n" socket.puts "\r\n" - headers = socket.readline("\r\n\r\n") - .split("\r\n") - .drop(1) - .map { |line| line.split(/:\s?/) } - .to_h + headers = header_hash socket content_length = headers["Content-Length"].to_i diff --git a/test/test_request_invalid.rb b/test/test_request_invalid.rb index a37bad40..6efdbeff 100644 --- a/test/test_request_invalid.rb +++ b/test/test_request_invalid.rb @@ -24,7 +24,7 @@ class TestRequestInvalid < Minitest::Test # this app should never be called, used for debugging app = ->(env) { - body = ''.dup + body = +'' env.each do |k,v| body << "#{k} = #{v}\n" if k == 'rack.input' diff --git a/test/test_worker_gem_independence.rb b/test/test_worker_gem_independence.rb index 085a0222..d064dfd4 100644 --- a/test/test_worker_gem_independence.rb +++ b/test/test_worker_gem_independence.rb @@ -103,7 +103,7 @@ class TestWorkerGemIndependence < TestIntegration initial_reply = read_body(connection) assert_equal old_version, initial_reply - before_restart.call if before_restart + before_restart&.call set_release_symlink File.expand_path(new_app_dir, __dir__) Dir.chdir(current_release_symlink) do