From aa4add41f3b5a55996a76095eaeb48daf2ead11e Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Sun, 30 Jun 2024 14:37:27 -0700 Subject: [PATCH] Fixes #27: upgrade gems, add rubocop plugins * Bump up Ruby versions * Deprecate Ruby 2.5 * Fix Rubocop errors * Bump version to 0.4.0 * Upgrade CodeCov; don't fail CI --- .envrc | 3 ++ .github/workflows/main.yml | 19 ++++------ .gitignore | 1 + .rubocop.yml | 15 +++++++- Gemfile.puma-v5 | 15 +++++++- Gemfile.puma-v6 | 15 +++++++- Makefile | 14 +++---- codecov.yml | 16 ++++++++ lib/puma/daemon/runner_adapter.rb | 7 ++-- lib/puma/daemon/version.rb | 2 +- puma-daemon.gemspec | 15 ++------ spec/puma/configuration_spec.rb | 49 +++++++++++++++++-------- spec/puma/daemon/runner_adapter_spec.rb | 2 +- spec/puma/daemon_spec.rb | 34 ++++++++--------- spec/puma/launch_spec.rb | 20 ++++------ spec/spec_helper.rb | 15 +++----- spec/support/puma_helpers.rb | 40 +++++++++++++++++--- 17 files changed, 184 insertions(+), 98 deletions(-) diff --git a/.envrc b/.envrc index 904f74d..c4ca671 100644 --- a/.envrc +++ b/.envrc @@ -1,2 +1,5 @@ alias be='bundle exec' alias bep='bundle exec puma -C config/puma.rb --debug -v --redirect-stderr puma.stderr --redirect-stdout puma.stdout spec/rackup/bind.ru' + + +[[ -f .envrc.local ]] && source .envrc.local diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b6d3c88..9db5ddb 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -7,7 +7,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - ruby-version: [3.2.0, 3.1.3, 3.0.5, 2.7.2, 2.6.6, 2.5.7] + ruby-version: [3.3.1, 3.2.4, 3.1.5, 3.0.7, 2.7.8, 2.6.10] env: RUBY_VERSION: ${{ matrix.ruby-version }} @@ -20,21 +20,18 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby-version }} + bundler-cache: true # runs 'bun - name: Run Specs run: make test-all - name: Rubocop - run: bundle exec rubocop + run: bundle exec rubocop --parallel - - uses: codecov/codecov-action@v3 + - uses: codecov/codecov-action@v4 with: - token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos - directory: coverage - env_vars: RUBY_VERSION - files: codecov-result.json - flags: unittests # optional - name: codecov-umbrella # optional fail_ci_if_error: false # optional (default = false) - verbose: true # optional (default = false) - + files: coverage/coverage.xml + flags: rspecs # optional + token: ${{ secrets.CODECOV_TOKEN }} # required + verbose: true # optional (default = false)` diff --git a/.gitignore b/.gitignore index 0e40804..23547ae 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,4 @@ /tmp/pids/* /Gemfile **/.DS_Store +.envrc.local diff --git a/.rubocop.yml b/.rubocop.yml index cba663c..47c339b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,9 +1,15 @@ +require: + - rubocop-performance + - rubocop-rake + - rubocop-rspec + inherit_gem: relaxed-rubocop: .rubocop.yml AllCops: - TargetRubyVersion: 2.4 NewCops: enable + SuggestExtensions: true + TargetRubyVersion: 2.6 Style/OptionalBooleanParameter: Exclude: @@ -24,5 +30,12 @@ Naming/FileName: Exclude: - 'lib/puma-daemon.rb' +Rake/Desc: + Enabled: false + +RSpec/MultipleMemoizedHelpers: + Max: 20 + + # Gemspec/DevelopmentDependencies: # Enabled: false diff --git a/Gemfile.puma-v5 b/Gemfile.puma-v5 index adb6892..3f02647 100644 --- a/Gemfile.puma-v5 +++ b/Gemfile.puma-v5 @@ -9,8 +9,21 @@ gem 'puma', '~> 5.0' gem 'rack', '~> 2' gem 'rake', '~> 13.0' +group :development, :test do + gem 'asciidoctor' + gem 'yard' +end + group :test do gem 'codecov', require: false + gem 'httparty' + gem 'relaxed-rubocop' gem 'rspec', '~> 3.0' - gem 'rubocop', '~> 0.80' + gem 'rspec-its' + gem 'rubocop' + gem 'rubocop-performance' + gem 'rubocop-rake' + gem 'rubocop-rspec' + gem 'simplecov' + gem 'simplecov-cobertura', require: false end diff --git a/Gemfile.puma-v6 b/Gemfile.puma-v6 index 91af7d1..2365040 100644 --- a/Gemfile.puma-v6 +++ b/Gemfile.puma-v6 @@ -9,8 +9,21 @@ gem 'puma', '~> 6.0' gem 'rack', '~> 2' gem 'rake', '~> 13.0' +group :development, :test do + gem 'asciidoctor' + gem 'yard' +end + group :test do gem 'codecov', require: false + gem 'httparty' + gem 'relaxed-rubocop' gem 'rspec', '~> 3.0' - gem 'rubocop', '~> 0.80' + gem 'rspec-its' + gem 'rubocop' + gem 'rubocop-performance' + gem 'rubocop-rake' + gem 'rubocop-rspec' + gem 'simplecov' + gem 'simplecov-cobertura', require: false end diff --git a/Makefile b/Makefile index 66b49c3..0b81165 100755 --- a/Makefile +++ b/Makefile @@ -1,9 +1,9 @@ -# vim: tabstop=8 -# vim: shiftwidth=8 +# vim: ts=8 +# vim: sw=8 # vim: noexpandtab # grep '^[a-z\-]*:' Makefile | cut -d: -f 1 | tr '\n' ' ' -.PHONY: help docker-build docker-bash +.PHONY: help docker-build docker-bash puma-v5 puma-v6 test test-all rubocop generate-pdf RUBY_VERSION := $(cat .ruby-version) OS := $(shell uname -s | tr '[:upper:]' '[:lower:]') @@ -14,7 +14,7 @@ CURRENT_DIR := $(notdir $(patsubst %/,%,$(dir $(MAKEFILE_PATH)))) PUMAD_HOME := $(shell dirname $(MAKEFILE_PATH)) help: ## Prints help message auto-generated from the comments. - @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' + @grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' puma-v5: ## Installs puma 5.0.0 @ln -nfs Gemfile.puma-v5 Gemfile @@ -37,7 +37,7 @@ rubocop: ## Run rubocop @bundle check || bundle install @bundle exec rubocop --format=progress - + docker-build-ruby: ## Builds the Docker image by compiling ruby 3.0.0 @docker build -f Dockerfile.build -t puma-daemon:latest . @@ -55,5 +55,5 @@ generate-pdf: ## Regenerates README,pdf from README.adoc @bash -c "source ~/.bashmatic/init.sh && ~/.bashmatic/bin/adoc2pdf README.adoc" clean: ## Clean-up - @rm -rf Gemfile Gemfile.lock coverage - + @rm -rf Gemfile Gemfile.lock coverage + diff --git a/codecov.yml b/codecov.yml index 5fea271..8d6aaa8 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,4 +1,20 @@ comment: layout: "reach,diff,flags,tree,betaprofiling" show_critical_paths: true +coverage: + status: + project: + default: # This can be anything, but it needs to exist as the name + # basic settings + target: auto + threshold: 5% + base: auto + if_ci_failed: error #success, failure, error, ignore + only_pulls: false + flags: + - rspecs + paths: + - lib + informational: true + removed_code_behavior: off diff --git a/lib/puma/daemon/runner_adapter.rb b/lib/puma/daemon/runner_adapter.rb index 8efbd61..c81e61c 100644 --- a/lib/puma/daemon/runner_adapter.rb +++ b/lib/puma/daemon/runner_adapter.rb @@ -4,6 +4,7 @@ module Puma module Daemon + # noinspection RubySuperCallWithoutSuperclassInspection module RunnerAdapter class << self def included(base) @@ -14,7 +15,7 @@ def included(base) base.class_eval do def output_header(mode) - super(mode) + super daemonize! if daemon? end @@ -35,10 +36,10 @@ def daemonize! end def log(str) - return if str =~ /Ctrl-C/ + return if str.include?('Ctrl-C') begin - super(str) + super rescue StandardError puts(str) end diff --git a/lib/puma/daemon/version.rb b/lib/puma/daemon/version.rb index a444938..ebc31bb 100644 --- a/lib/puma/daemon/version.rb +++ b/lib/puma/daemon/version.rb @@ -2,6 +2,6 @@ module Puma module Daemon - VERSION = '0.3.2' + VERSION = '0.4.0' end end diff --git a/puma-daemon.gemspec b/puma-daemon.gemspec index d7ee286..3472a2b 100644 --- a/puma-daemon.gemspec +++ b/puma-daemon.gemspec @@ -11,10 +11,10 @@ Gem::Specification.new do |spec| spec.summary = "Restore somewhat Puma's ability to self-daemonize, since Puma 5.0 dropped it" spec.description = <<~DESCRIPTION - In version 5.0 the authors of the popular Ruby web server Puma chose to remove the + In version 5.0 the authors of the popular Ruby web server Puma chose to remove the#{' '} daemonization support from Puma, because the code wasn't wall maintained, and because other and better options exist for production deployments. For example - systemd, Docker/Kubernetes, Heroku, etc. + systemd, Docker/Kubernetes, Heroku, etc.#{' '} Having said that, it was neat and often useful to daemonize Puma in development. This gem adds this support to Puma 5 & 6 (hopefully) without breaking anything in Puma @@ -27,7 +27,7 @@ Gem::Specification.new do |spec| spec.homepage = 'https://github.com/kigster/puma-daemon' spec.license = 'MIT' - spec.required_ruby_version = Gem::Requirement.new('>= 2.4.0') + spec.required_ruby_version = Gem::Requirement.new('>= 2.6') spec.metadata['homepage_uri'] = spec.homepage spec.metadata['source_code_uri'] = 'https://github.com/kigster/puma-daemon' @@ -46,15 +46,6 @@ Gem::Specification.new do |spec| spec.add_dependency 'puma', '>= 5.0' spec.add_dependency 'rack' - spec.add_development_dependency 'asciidoctor' - spec.add_development_dependency 'codecov' - spec.add_development_dependency 'httparty' - spec.add_development_dependency 'relaxed-rubocop' - spec.add_development_dependency 'rspec-its' - spec.add_development_dependency 'rubocop' - spec.add_development_dependency 'simplecov' - spec.add_development_dependency 'yard' - # For more information and examples about making a new gem, checkout our # guide at: https://bundler.io/guides/creating_gem.html spec.metadata['rubygems_mfa_required'] = 'true' diff --git a/spec/puma/configuration_spec.rb b/spec/puma/configuration_spec.rb index e643dc3..2e1aff6 100644 --- a/spec/puma/configuration_spec.rb +++ b/spec/puma/configuration_spec.rb @@ -14,28 +14,34 @@ module Puma end end - it(:puma_default_options) { should_not be_nil } + it(:puma_default_options) { is_expected.not_to be_nil } - context 'default_dsl' do + describe '#default_dsl' do subject { config.default_dsl.options } - its([:daemon]) { should be true } - end - context 'user_dsl' do - subject { config.user_dsl.options } - its([:daemon]) { should be true } - its([:binds]) { should eq ['tcp://0.0.0.0:3001'] } + its([:daemon]) { is_expected.to be true } end - describe 'config: daemonize(false)' do - let(:daemonize) { false } - context 'user_dsl' do + describe '#user_dsl' do + describe 'when using default options' do subject { config.user_dsl.options } - its([:daemon]) { should be false } + + its([:daemon]) { is_expected.to be true } + its([:binds]) { is_expected.to eq ['tcp://0.0.0.0:3001'] } + end + + describe 'when daemonize is false)' do + subject { config.user_dsl.options } + + let(:daemonize) { false } + + its([:daemon]) { is_expected.to be false } end end - context 'file_dsl' do + describe 'file_dsl' do + subject { config.options } + let(:config) do ::Puma::Configuration.new do |c| c.rackup 'spec/rackup/bind.ru' @@ -43,10 +49,23 @@ module Puma end.tap(&:load) end + its([:binds]) { is_expected.to eq ['tcp://0.0.0.0:3000'] } + its([:daemon]) { is_expected.to be true } + end + + describe 'pidfile' do subject { config.options } - its([:binds]) { should eq ['tcp://0.0.0.0:3000'] } - its([:daemon]) { should be true } + let(:pidfile) { Tempfile.new('pidfile') } + let(:config) do + ::Puma::Configuration.new do |c| + c.pidfile(pidfile) + c.port 3000 + end.tap(&:load) + end + + its([:binds]) { is_expected.to eq ['tcp://0.0.0.0:3000'] } + its([:daemon]) { is_expected.to be true } end end end diff --git a/spec/puma/daemon/runner_adapter_spec.rb b/spec/puma/daemon/runner_adapter_spec.rb index 936dbcd..534a05f 100644 --- a/spec/puma/daemon/runner_adapter_spec.rb +++ b/spec/puma/daemon/runner_adapter_spec.rb @@ -17,7 +17,7 @@ module Daemon before { adapter.has_demonized = false - expect(::Process).to receive(:daemon) + allow(::Process).to receive(:daemon) } its(:daemonize!) { is_expected.to be_truthy } diff --git a/spec/puma/daemon_spec.rb b/spec/puma/daemon_spec.rb index 81cabe1..a113028 100644 --- a/spec/puma/daemon_spec.rb +++ b/spec/puma/daemon_spec.rb @@ -5,15 +5,15 @@ module Puma RSpec.describe Daemon do - it 'has a version number' do - expect(Daemon::VERSION).not_to be nil - end - - let(:environment) { 'production' } - let(:wait_booted) { -> { wait.sysread 1 } } + let(:cli) { ::Puma::Daemon::CLI.new(argv).cli } let(:argv) { [] } + let(:wait_booted) { -> { wait.sysread 1 } } + let(:environment) { 'production' } - include TmpPath + after do + @wait&.close + @ready&.close + end before do @environment = 'production' @@ -22,33 +22,32 @@ module Puma @tmp_path1 = tmp_path('puma-test-1') @tmp_path2 = tmp_path('puma-test-2') - File.unlink @tmp_path1 if File.exist? @tmp_path1 - File.unlink @tmp_path2 if File.exist? @tmp_path2 + FileUtils.rm_f @tmp_path1 + FileUtils.rm_f @tmp_path2 @wait, @ready = IO.pipe end - after do - @wait&.close - @ready&.close + it 'has a version number' do + expect(Daemon::VERSION).not_to be_nil end - let(:cli) { ::Puma::Daemon::CLI.new(argv).cli } + include TmpPath - context 'runners' do + describe 'when starting Runners' do describe 'single-process daemon' do let(:argv) { [] } describe Single do - subject(:single) { ::Puma::Const::VERSION =~ /^5/ ? described_class.new(cli.launcher, cli.instance_variable_get(:@events)) : described_class.new(cli.launcher) } + subject(:single) { ::Puma::Const::VERSION.match?(/^5/) ? described_class.new(cli.launcher, cli.instance_variable_get(:@events)) : described_class.new(cli.launcher) } # This is not a real test it { is_expected.to respond_to :daemonize! } end end - describe 'multi-process cluster daemon' do + describe 'when using multi-process cluster daemon' do describe Cluster do - subject(:single) { ::Puma::Const::VERSION =~ /^5/ ? described_class.new(cli.launcher, cli.instance_variable_get(:@events)) : described_class.new(cli.launcher) } + subject(:single) { ::Puma::Const::VERSION.match?(/^5/) ? described_class.new(cli.launcher, cli.instance_variable_get(:@events)) : described_class.new(cli.launcher) } let(:url) { "unix://#{@tmp_path1}" } let(:argv) do ['-b', "unix://#{@tmp_path2}", @@ -58,6 +57,7 @@ module Puma '--control-token', '', 'spec/rackup/lobster.ru'] end + # This is not a real test it { is_expected.to respond_to :daemonize! } end diff --git a/spec/puma/launch_spec.rb b/spec/puma/launch_spec.rb index 15eed02..17a0574 100644 --- a/spec/puma/launch_spec.rb +++ b/spec/puma/launch_spec.rb @@ -26,7 +26,7 @@ def start_server_and_verify!(port, cli) Thread.join - expect(response).to_not be_nil + expect(response).not_to be_nil expect(response.code).to eq(200) expect(response.body).to eq('Hello World') @@ -51,8 +51,8 @@ def start_server_and_verify!(port, cli) @port1 = 61_234 @port2 = 61_235 - File.unlink @tmp_path1 if File.exist? @tmp_path1 - File.unlink @tmp_path2 if File.exist? @tmp_path2 + FileUtils.rm_f @tmp_path1 + FileUtils.rm_f @tmp_path2 @wait, @ready = IO.pipe end @@ -62,9 +62,9 @@ def start_server_and_verify!(port, cli) @ready&.close end - context 'runners' do + describe 'runner' do describe 'single-process daemon' do - describe Single do + describe 'Puma::Single' do let(:argv) { %W[-b tcp://0.0.0.0:#{@port1} spec/rackup/bind.ru] } let(:cli) { ::Puma::Daemon::CLI.new(argv).cli } let(:runner) { runner_instance(cli) } @@ -72,11 +72,8 @@ def start_server_and_verify!(port, cli) before { ::RSpec::PumaHelpers.puma_kill[] } - it 'should have puma running on the background' do + it 'has puma running on the background' do start_server_and_verify!(port, cli) - end - - after do expect(::RSpec::PumaHelpers.puma_pids[]).to be_empty end end @@ -89,11 +86,8 @@ def start_server_and_verify!(port, cli) before { ::RSpec::PumaHelpers.puma_kill[] } - it 'should have puma running on the background' do + it 'has puma running on the background' do start_server_and_verify!(port, cli) - end - - after do expect(::RSpec::PumaHelpers.puma_pids[]).to be_empty end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7e82f27..f36dc43 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,16 +2,10 @@ require 'rspec' require 'rspec/its' -require 'simplecov' - -if ENV['CODECOV_TOKEN'] - require 'codecov' - SimpleCov.formatter = SimpleCov::Formatter::Codecov -end +require 'tempfile' -SimpleCov.enable_for_subprocesses true -SimpleCov.use_merging true -SimpleCov.merge_timeout(4000) +require 'simplecov' +require 'simplecov-cobertura' SimpleCov.at_fork do |pid| # This needs a unique name so it won't be overwritten @@ -22,7 +16,8 @@ # start SimpleCov.formatter SimpleCov::Formatter::MultiFormatter.new([ SimpleCov::Formatter::SimpleFormatter, - SimpleCov::Formatter::HTMLFormatter + SimpleCov::Formatter::HTMLFormatter, + SimpleCov::Formatter::CoberturaFormatter ]) SimpleCov.start SimpleCov.track_files 'lib/**/*.rb' diff --git a/spec/support/puma_helpers.rb b/spec/support/puma_helpers.rb index 2c89839..522d55b 100644 --- a/spec/support/puma_helpers.rb +++ b/spec/support/puma_helpers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'puma/daemon' - +require 'open3' module RSpec module PumaHelpers HAS_FORK = ::Process.respond_to? :fork @@ -20,7 +20,7 @@ def skip_sockets end def runner_instance(cli) - ::Puma::Const::VERSION =~ /^5/ ? described_class.new(cli.launcher, cli.instance_variable_get(:@events)) : described_class.new(cli.launcher) + ::Puma::Const::VERSION.match?(/^5/) ? described_class.new(cli.launcher, cli.instance_variable_get(:@events)) : described_class.new(cli.launcher) end def fork!(input: '') @@ -65,21 +65,51 @@ def fork!(input: '') [pipes[:stdout][0].read, pipes[:stderr][0].read] end + PROCESS_CMD = "bash -c \"ps -ef | grep [p]uma | grep -v -E 'rspec|rubocop|Makefile|Rakefile'\" 2>&1" + def puma_pids - lambda { `ps -ef | grep [p]uma | grep -v rspec | awk '{print $2}'`.split(/\n/) } + lambda do + stdout, code = Open3.capture2("#{PROCESS_CMD} | awk '{print $2}'") + stdout&.split("\n") if code == 0 + end + end + + def puma_pid_processes + lambda do + stdout, code = Open3.capture2(PROCESS_CMD) + stdout&.split("\n") if code == 0 + end end def puma_kill lambda do + process_descriptions = puma_pid_processes[] + + if process_descriptions.nil? || process_descriptions.empty? + puts 'NOTE: no child processes found after fork()' + return + end + + current_pid = Process.pid + + warn "Detected the following forked processes (current pid is #{current_pid})" + warn '———————————————————————————————————————————————————' + warn process_descriptions.join("\n") + warn '———————————————————————————————————————————————————' + puma_pids[].each do |pid| + next if pid == current_pid + warn "killing puma pid #{pid}" + begin - Process.kill('TERM', pid.to_i) + Process.kill('SIGTERM', pid.to_i) rescue StandardError nil end + begin - Process.kill('KILL', pid.to_i) + Process.kill('SIGKILL', pid.to_i) rescue StandardError nil end