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

@@ -2,6 +2,7 @@
 .ruby-version
 tags
 Gemfile.lock
+gemfiles/*.lock
 *.swp
 dump.rdb
 .rbx

.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

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

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

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

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: "../"

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: "../"

lib/sidekiq/cli.rb

@@ -24,7 +24,6 @@ module Sidekiq
       proc { |me, data| "stopping" if me.stopping? },
     ]
 
-    # Used for CLI testing
     attr_accessor :launcher
     attr_accessor :environment
 
@@ -45,7 +44,7 @@ module Sidekiq
       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 @@ module Sidekiq
       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 @@ module Sidekiq
     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 @@ module Sidekiq
     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 @@ module Sidekiq
         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
 

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 @@ if defined?(::Rails) && ::Rails::VERSION::MAJOR < 4
   $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
+end

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

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

test/dummy/config/environment.rb

@@ -0,0 +1,3 @@
+require_relative 'application'
+
+Rails.application.initialize!

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 @@ if ENV["COVERAGE"]
   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

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])
-
-      refute($LOADED_FEATURES.any? { |x| x =~ /fake_env/ })
+  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
 
-    it 'does not boot rails' do
-      refute defined?(::Rails::Application)
+    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
 
-      @cli.parse(%w[sidekiq -r ./myapp])
+        assert_equal Process.pid, File.read('/tmp/sidekiq.pid').chop.to_i
+      end
+    end
 
-      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
+
+          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
+
+          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
 

test/test_extensions.rb

@@ -8,7 +8,6 @@ Sidekiq::Extensions.enable_delay!
 class TestExtensions < Minitest::Test
   describe 'sidekiq extensions' do
     before do
-      Sidekiq.redis = REDIS
       Sidekiq.redis {|c| c.flushdb }
     end
 

test/test_fetch.rb

@@ -6,17 +6,12 @@ require 'sidekiq/api'
 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

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

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 @@ class TestRedisConnection < Minitest::Test
     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 @@ class TestRedisConnection < Minitest::Test
         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