From ea9b34a953c4ce23bf2f9c2b2c7056139529e58e Mon Sep 17 00:00:00 2001 From: Andrew Babichev Date: Fri, 7 Dec 2018 17:59:42 +0100 Subject: [PATCH] Add CLI Run Test - Write Pid, System Boot, Print Banner (#4039) * Add appraisal * Test CLI run (write pid, system boot, print banner) * Cleanup test helper * Set REDIS_URL to use non-default host in test env * Move mintiest-focus to test bundle group --- .gitignore | 1 + .travis.yml | 7 ++- Appraisals | 9 ++++ Gemfile | 28 ++++++++--- Rakefile | 3 +- gemfiles/rails_4.gemfile | 31 +++++++++++++ gemfiles/rails_5.gemfile | 31 +++++++++++++ lib/sidekiq/cli.rb | 29 +++++------- lib/sidekiq/rails.rb | 3 +- test/dummy/config/application.rb | 15 ++++++ test/dummy/config/database.yml | 11 +++++ test/dummy/config/environment.rb | 3 ++ test/helper.rb | 37 ++++----------- test/test_cli.rb | 79 ++++++++++++++++++++++++++++---- test/test_extensions.rb | 1 - test/test_fetch.rb | 5 -- test/test_middleware.rb | 1 - test/test_redis_connection.rb | 11 ++++- 18 files changed, 229 insertions(+), 76 deletions(-) create mode 100644 Appraisals create mode 100644 gemfiles/rails_4.gemfile create mode 100644 gemfiles/rails_5.gemfile create mode 100644 test/dummy/config/application.rb create mode 100644 test/dummy/config/database.yml create mode 100644 test/dummy/config/environment.rb diff --git a/.gitignore b/.gitignore index 270a49e49..29d648687 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .ruby-version tags Gemfile.lock +gemfiles/*.lock *.swp dump.rdb .rbx diff --git a/.travis.yml b/.travis.yml index b17f59308..6a0bca025 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,8 +4,11 @@ cache: bundler services: - redis-server before_install: - - gem install bundler - - gem update bundler + - gem update --system +gemfile: + - gemfiles/rails_4.gemfile + - gemfiles/rails_5.gemfile +bundler_args: --without development load_test rvm: - 2.2.10 - 2.3.7 diff --git a/Appraisals b/Appraisals new file mode 100644 index 000000000..fe42abd4c --- /dev/null +++ b/Appraisals @@ -0,0 +1,9 @@ +appraise "rails-4" do + gem "rails", "~> 4.2" + gem 'activerecord-jdbcsqlite3-adapter', '< 50', platforms: :jruby +end + +appraise "rails-5" do + gem "rails", "~> 5.2" + gem 'activerecord-jdbcsqlite3-adapter', '>= 50', platforms: :jruby +end diff --git a/Gemfile b/Gemfile index 78ffde058..c075c91e5 100644 --- a/Gemfile +++ b/Gemfile @@ -1,15 +1,29 @@ source 'https://rubygems.org' + gemspec -# load testing -#gem "hiredis" -#gem 'toxiproxy' +gem 'rake' +gem 'redis-namespace' +gem 'rails', '~> 5.2' +gem 'sqlite3', platforms: :ruby +gem 'activerecord-jdbcsqlite3-adapter', platforms: :jruby + +group :development do + gem 'appraisal' +end group :test do - gem 'rails', '>= 5.0.1' gem 'minitest' - gem 'pry-byebug', platforms: :mri - gem 'rake' - gem 'redis-namespace' + gem 'minitest-focus' + gem 'minitest-reporters' gem 'simplecov' end + +group :development, :test do + gem 'pry-byebug', platforms: :mri +end + +group :load_test do + gem 'hiredis' + gem 'toxiproxy' +end diff --git a/Rakefile b/Rakefile index 5a30d2082..4e5589e1f 100644 --- a/Rakefile +++ b/Rakefile @@ -1,8 +1,9 @@ require 'bundler/gem_tasks' require 'rake/testtask' + Rake::TestTask.new(:test) do |test| test.warning = true test.pattern = 'test/**/test_*.rb' end -task :default => :test +task default: :test diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile new file mode 100644 index 000000000..22678d11e --- /dev/null +++ b/gemfiles/rails_4.gemfile @@ -0,0 +1,31 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rake" +gem "redis-namespace" +gem "rails", "~> 4.2" +gem "sqlite3", platforms: :ruby +gem "activerecord-jdbcsqlite3-adapter", "< 50", platforms: :jruby + +group :development do + gem "appraisal" +end + +group :test do + gem "minitest" + gem "minitest-focus" + gem "minitest-reporters" + gem "simplecov" +end + +group :development, :test do + gem "pry-byebug", platforms: :mri +end + +group :load_test do + gem "hiredis" + gem "toxiproxy" +end + +gemspec path: "../" diff --git a/gemfiles/rails_5.gemfile b/gemfiles/rails_5.gemfile new file mode 100644 index 000000000..a3377873c --- /dev/null +++ b/gemfiles/rails_5.gemfile @@ -0,0 +1,31 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rake" +gem "redis-namespace" +gem "rails", "~> 5.2" +gem "sqlite3", platforms: :ruby +gem "activerecord-jdbcsqlite3-adapter", ">= 50", platforms: :jruby + +group :development do + gem "appraisal" +end + +group :test do + gem "minitest" + gem "minitest-focus" + gem "minitest-reporters" + gem "simplecov" +end + +group :development, :test do + gem "pry-byebug", platforms: :mri +end + +group :load_test do + gem "hiredis" + gem "toxiproxy" +end + +gemspec path: "../" diff --git a/lib/sidekiq/cli.rb b/lib/sidekiq/cli.rb index 1c1d6a2a2..aaa87c322 100644 --- a/lib/sidekiq/cli.rb +++ b/lib/sidekiq/cli.rb @@ -24,7 +24,6 @@ class CLI proc { |me, data| "stopping" if me.stopping? }, ] - # Used for CLI testing attr_accessor :launcher attr_accessor :environment @@ -45,7 +44,7 @@ def run daemonize if options[:daemon] write_pid boot_system - print_banner + print_banner if environment == 'development' && $stdout.tty? self_read, self_write = IO.pipe sigs = %w(INT TERM TTIN TSTP) @@ -93,6 +92,10 @@ def run logger.debug { "Client Middleware: #{Sidekiq.client_middleware.map(&:klass).join(', ')}" } logger.debug { "Server Middleware: #{Sidekiq.server_middleware.map(&:klass).join(', ')}" } + launch(self_read) + end + + def launch(self_read) if !options[:daemon] logger.info 'Starting processing, hit Ctrl-C to stop' end @@ -178,21 +181,16 @@ def handle_signal(sig) private def print_banner - # Print logo and banner for development - if environment == 'development' && $stdout.tty? - puts "\e[#{31}m" - puts Sidekiq::CLI.banner - puts "\e[0m" - end + puts "\e[#{31}m" + puts Sidekiq::CLI.banner + puts "\e[0m" end def daemonize raise ArgumentError, "You really should set a logfile if you're going to daemonize" unless options[:logfile] - files_to_reopen = [] - ObjectSpace.each_object(File) do |file| - files_to_reopen << file unless file.closed? - end + files_to_reopen = ObjectSpace.each_object(File).reject { |f| f.closed? } + ::Process.daemon(true, true) files_to_reopen.each do |file| @@ -251,8 +249,6 @@ def options def boot_system ENV['RACK_ENV'] = ENV['RAILS_ENV'] = environment - raise ArgumentError, "#{options[:require]} does not exist" unless File.exist?(options[:require]) - if File.directory?(options[:require]) require 'rails' if ::Rails::VERSION::MAJOR < 4 @@ -272,10 +268,7 @@ def boot_system end options[:tag] ||= default_tag else - not_required_message = "#{options[:require]} was not required, you should use an explicit path: " + - "./#{options[:require]} or /path/to/#{options[:require]}" - - require(options[:require]) || raise(ArgumentError, not_required_message) + require options[:require] end end diff --git a/lib/sidekiq/rails.rb b/lib/sidekiq/rails.rb index f40f77282..773ce99ab 100644 --- a/lib/sidekiq/rails.rb +++ b/lib/sidekiq/rails.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Sidekiq class Rails < ::Rails::Engine # We need to setup this up before any application configuration which might @@ -54,4 +55,4 @@ def inspect $stderr.puts("**************************************************") $stderr.puts("⛔️ WARNING: Sidekiq server is no longer supported by Rails 3.2 - please ensure your server/workers are updated") $stderr.puts("**************************************************") -end \ No newline at end of file +end diff --git a/test/dummy/config/application.rb b/test/dummy/config/application.rb new file mode 100644 index 000000000..5c73c4ca9 --- /dev/null +++ b/test/dummy/config/application.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require "rails/all" + +module Dummy + class Application < Rails::Application + config.root = File.expand_path("../..", __FILE__) + config.eager_load = false + config.logger = Logger.new('/dev/null') + + if Rails::VERSION::MAJOR >= 5 + config.active_record.sqlite3.represent_boolean_as_integer = true + end + end +end diff --git a/test/dummy/config/database.yml b/test/dummy/config/database.yml new file mode 100644 index 000000000..29cc2b2f0 --- /dev/null +++ b/test/dummy/config/database.yml @@ -0,0 +1,11 @@ +development: + adapter: sqlite3 + database: db/development.sqlite3 + pool: 5 + timeout: 5000 + +test: + adapter: sqlite3 + database: db/test.sqlite3 + pool: 5 + timeout: 5000 diff --git a/test/dummy/config/environment.rb b/test/dummy/config/environment.rb new file mode 100644 index 000000000..875ab0f2e --- /dev/null +++ b/test/dummy/config/environment.rb @@ -0,0 +1,3 @@ +require_relative 'application' + +Rails.application.initialize! diff --git a/test/helper.rb b/test/helper.rb index de105bd64..da80a46a9 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -1,7 +1,12 @@ # frozen_string_literal: true -require "bundler/setup" -Bundler.require +require 'bundler/setup' +Bundler.require(:default, :test) + +require 'minitest/reporters' +require 'minitest/autorun' + +MiniTest::Reporters.use! Minitest::Reporters::DefaultReporter.new $TESTING = true # disable minitest/parallel threads @@ -15,36 +20,10 @@ end end -ENV['RACK_ENV'] = ENV['RAILS_ENV'] = 'test' - -trap 'TSTP' do - threads = Thread.list - - puts - puts "=" * 80 - puts "Received TSTP signal; printing all #{threads.count} thread backtraces." - - threads.each do |thr| - description = thr == Thread.main ? "Main thread" : thr.inspect - puts - puts "#{description} backtrace: " - puts thr.backtrace.join("\n") - end - - puts "=" * 80 -end - -require 'minitest/autorun' +ENV['REDIS_URL'] ||= 'redis://localhost/15' Sidekiq.logger.level = Logger::ERROR -REDIS_URL = ENV['REDIS_URL'] || 'redis://localhost/15' -REDIS = Sidekiq::RedisConnection.create(:url => REDIS_URL) - -Sidekiq.configure_client do |config| - config.redis = { :url => REDIS_URL } -end - def capture_logging(lvl=Logger::INFO) old = Sidekiq.logger begin diff --git a/test/test_cli.rb b/test/test_cli.rb index 5cb944a0d..9fea92f5f 100644 --- a/test/test_cli.rb +++ b/test/test_cli.rb @@ -2,16 +2,15 @@ require_relative 'helper' require 'sidekiq/cli' -require 'tempfile' class TestCLI < Minitest::Test describe '#parse' do before do - @cli = Sidekiq::CLI.new Sidekiq.options = Sidekiq::DEFAULTS.dup @logger = Sidekiq.logger @logdev = StringIO.new Sidekiq.logger = Logger.new(@logdev) + @cli = Sidekiq::CLI.new end after do @@ -263,19 +262,81 @@ class TestCLI < Minitest::Test end end end + end - it 'does not require the specified Ruby code' do - @cli.parse(%w[sidekiq -r ./test/fake_env.rb]) + describe '#run' do + before do + Sidekiq.options = Sidekiq::DEFAULTS.dup + Sidekiq.options[:require] = './test/fake_env.rb' + @logger = Sidekiq.logger + @logdev = StringIO.new + Sidekiq.logger = Logger.new(@logdev) + @cli = Sidekiq::CLI.new + end + + after do + Sidekiq.logger = @logger + end + + describe 'pidfile' do + it 'writes process pid to file' do + Sidekiq.options[:pidfile] = '/tmp/sidekiq.pid' + @cli.stub(:launch, nil) do + @cli.run + end - refute($LOADED_FEATURES.any? { |x| x =~ /fake_env/ }) + assert_equal Process.pid, File.read('/tmp/sidekiq.pid').chop.to_i + end end - it 'does not boot rails' do - refute defined?(::Rails::Application) + describe 'require workers' do + describe 'when path is a rails directory' do + before do + Sidekiq.options[:require] = './test/dummy' + @cli.environment = 'test' + end + + it 'requires sidekiq railtie and rails application with environment' do + @cli.stub(:launch, nil) do + @cli.run + end - @cli.parse(%w[sidekiq -r ./myapp]) + assert defined?(Sidekiq::Rails) + assert defined?(Dummy::Application) + end + + it 'tags with the app directory name' do + @cli.stub(:launch, nil) do + @cli.run + end - refute defined?(::Rails::Application) + assert_equal 'dummy', Sidekiq.options[:tag] + end + end + + describe 'when path is file' do + it 'requires application' do + @cli.stub(:launch, nil) do + @cli.run + end + + assert $LOADED_FEATURES.any? { |x| x =~ /test\/fake_env/ } + end + end + end + + describe 'when development environment and stdout tty' do + it 'prints banner' do + @cli.stub(:environment, 'development') do + assert_output(/#{Regexp.escape(Sidekiq::CLI.banner)}/) do + $stdout.stub(:tty?, true) do + @cli.stub(:launch, nil) do + @cli.run + end + end + end + end + end end end diff --git a/test/test_extensions.rb b/test/test_extensions.rb index 33f67dbc8..183a64250 100644 --- a/test/test_extensions.rb +++ b/test/test_extensions.rb @@ -8,7 +8,6 @@ class TestExtensions < Minitest::Test describe 'sidekiq extensions' do before do - Sidekiq.redis = REDIS Sidekiq.redis {|c| c.flushdb } end diff --git a/test/test_fetch.rb b/test/test_fetch.rb index 38da92450..503eabe26 100644 --- a/test/test_fetch.rb +++ b/test/test_fetch.rb @@ -6,17 +6,12 @@ class TestFetcher < Minitest::Test describe 'fetcher' do before do - Sidekiq.redis = { :url => REDIS_URL } Sidekiq.redis do |conn| conn.flushdb conn.rpush('queue:basic', 'msg') end end - after do - Sidekiq.redis = REDIS - end - it 'retrieves' do fetch = Sidekiq::BasicFetch.new(:queues => ['basic', 'bar']) uow = fetch.retrieve_work diff --git a/test/test_middleware.rb b/test/test_middleware.rb index 7c6569362..ff5066fc8 100644 --- a/test/test_middleware.rb +++ b/test/test_middleware.rb @@ -7,7 +7,6 @@ class TestMiddleware < Minitest::Test describe 'middleware chain' do before do $errors = [] - Sidekiq.redis = REDIS end class CustomMiddleware diff --git a/test/test_redis_connection.rb b/test/test_redis_connection.rb index ce8016379..2af17676b 100644 --- a/test/test_redis_connection.rb +++ b/test/test_redis_connection.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require_relative 'helper' require 'sidekiq/cli' @@ -6,6 +7,12 @@ class TestRedisConnection < Minitest::Test describe ".create" do before do Sidekiq.options = Sidekiq::DEFAULTS.dup + @old = ENV['REDIS_URL'] + ENV['REDIS_URL'] = 'redis://localhost/15' + end + + after do + ENV['REDIS_URL'] = @old end # To support both redis-rb 3.3.x #client and 4.0.x #_client @@ -81,7 +88,7 @@ def server_connection(*args) it "disables client setname with nil id" do pool = Sidekiq::RedisConnection.create(:id => nil) assert_equal Redis, pool.checkout.class - assert_equal "redis://127.0.0.1:6379/0", pool.checkout.connection.fetch(:id) + assert_equal "redis://localhost:6379/15", pool.checkout.connection.fetch(:id) end describe "network_timeout" do @@ -123,7 +130,7 @@ def server_connection(*args) pool = Sidekiq::RedisConnection.create(:path => "/var/run/redis.sock") assert_equal "unix", client_for(pool.checkout).scheme assert_equal "/var/run/redis.sock", pool.checkout.connection.fetch(:location) - assert_equal 0, pool.checkout.connection.fetch(:db) + assert_equal 15, pool.checkout.connection.fetch(:db) end it "uses a given :path and :db" do