Skip to content

Commit

Permalink
Cleanup CLI (#4026)
Browse files Browse the repository at this point in the history
* Refactor CLI test

Extracted 3 main parts:
- parse
- run
- signal handling

* Move demonization and pid write from parse to run phase

* Move queues default from validate to setup options phase

* Add pry-byebug gem

* Drop Sidekiq::Test

* Require launcher in CLI

* Remove TODOs
  • Loading branch information
Tensho authored and mperham committed Dec 3, 2018
1 parent 78f3b68 commit ddb0c8b
Show file tree
Hide file tree
Showing 35 changed files with 288 additions and 390 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ vendor/
.sass-cache/
tmp/
pkg/*.gem
.byebug_history
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ gemspec
group :test do
gem 'rails', '>= 5.0.1'
gem 'minitest'
gem 'pry-byebug', platforms: :mri
gem 'rake'
gem 'redis-namespace'
gem 'simplecov'
Expand Down
2 changes: 2 additions & 0 deletions lib/sidekiq.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# frozen_string_literal: true

require 'sidekiq/version'
fail "Sidekiq #{Sidekiq::VERSION} does not support Ruby versions below 2.2.2." if RUBY_PLATFORM != 'java' && Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.2')

Expand Down Expand Up @@ -56,6 +57,7 @@ def self.❨╯°□°❩╯︵┻━┻
def self.options
@options ||= DEFAULTS.dup
end

def self.options=(opts)
@options = opts
end
Expand Down
19 changes: 5 additions & 14 deletions lib/sidekiq/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

require 'sidekiq'
require 'sidekiq/util'
require 'sidekiq/launcher'

module Sidekiq
class CLI
Expand All @@ -24,22 +25,13 @@ class CLI
]

# Used for CLI testing
attr_accessor :code
attr_accessor :launcher
attr_accessor :environment

def initialize
@code = nil
end

def parse(args=ARGV)
@code = nil

setup_options(args)
initialize_logger
validate!
daemonize
write_pid
end

def jruby?
Expand All @@ -50,6 +42,8 @@ def jruby?
# global process state irreversibly. PRs which improve the
# test coverage of Sidekiq::CLI are welcomed.
def run
daemonize if options[:daemon]
write_pid
boot_system
print_banner

Expand Down Expand Up @@ -103,7 +97,6 @@ def run
logger.info 'Starting processing, hit Ctrl-C to stop'
end

require 'sidekiq/launcher'
@launcher = Sidekiq::Launcher.new(options)

begin
Expand Down Expand Up @@ -194,8 +187,6 @@ def print_banner
end

def daemonize
return unless options[:daemon]

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|
Expand Down Expand Up @@ -242,6 +233,8 @@ def setup_options(args)
opts = parse_options(args)
set_environment opts[:environment]

options[:queues] << 'default' if options[:queues].empty?

cfile = opts[:config_file]
opts = parse_config(cfile).merge(opts) if cfile

Expand Down Expand Up @@ -298,8 +291,6 @@ def default_tag
end

def validate!
options[:queues] << 'default' if options[:queues].empty?

if !File.exist?(options[:require]) ||
(File.directory?(options[:require]) && !File.exist?("#{options[:require]}/config/application.rb"))
logger.info "=================================================================="
Expand Down
1 change: 1 addition & 0 deletions test/config_empty.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
---
3 changes: 2 additions & 1 deletion test/env_based_config.yml → test/config_environment.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
---
:pidfile: /tmp/sidekiq-config-test.pid
:concurrency: 50

staging:
:verbose: false
:require: ./test/fake_env.rb
:logfile: /tmp/sidekiq.log
:concurrency: 5
:concurrency: 50
:queues:
- [<%="very_"%>often, 2]
- [seldom, 1]
File renamed without changes.
15 changes: 5 additions & 10 deletions test/helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
# frozen_string_literal: true

require "bundler/setup"
Bundler.require

$TESTING = true
# disable minitest/parallel threads
ENV["N"] = "0"
Expand All @@ -10,6 +14,7 @@
add_filter "/myapp/"
end
end

ENV['RACK_ENV'] = ENV['RAILS_ENV'] = 'test'

trap 'TSTP' do
Expand All @@ -29,20 +34,10 @@
puts "=" * 80
end

begin
require 'pry-byebug'
rescue LoadError
end

require 'minitest/autorun'

require 'sidekiq'
require 'sidekiq/util'
Sidekiq.logger.level = Logger::ERROR

Sidekiq::Test = Minitest::Test

require 'sidekiq/redis_connection'
REDIS_URL = ENV['REDIS_URL'] || 'redis://localhost/15'
REDIS = Sidekiq::RedisConnection.create(:url => REDIS_URL)

Expand Down
2 changes: 1 addition & 1 deletion test/test_actors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
require 'sidekiq/scheduled'
require 'sidekiq/processor'

class TestActors < Sidekiq::Test
class TestActors < Minitest::Test
class JoeWorker
include Sidekiq::Worker
def perform(slp)
Expand Down
2 changes: 1 addition & 1 deletion test/test_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require 'active_job'
require 'action_mailer'

class TestApi < Sidekiq::Test
class TestApi < Minitest::Test
describe 'api' do
before do
Sidekiq.redis {|c| c.flushdb }
Expand Down
Loading

0 comments on commit ddb0c8b

Please sign in to comment.