Fix: Strict ordering must be followed for queues without weights specified by -q (#4554)

Co-authored-by: Mike Perham <mperham@gmail.com>

Changes.md

@@ -9,6 +9,7 @@ Unreleased
 - Ensure `Rack::ContentLength` is loaded as middleware for correct Web UI responses [#4541]
 - Avoid exception dumping SSL store in Redis connection logging [#4532]
 - Better error messages in Sidekiq::Client [#4549]
+- Ensure `strict` order is followed when queues without weights are specified via the `-q` option [#4554]
 - Refactor `Sidekiq::CLI` to remove a redundant `.empty?` method call [#4552]
 
 6.0.7

lib/sidekiq.rb

@@ -20,6 +20,7 @@ module Sidekiq
     labels: [],
     concurrency: 10,
     require: ".",
+    strict: true,
     environment: nil,
     timeout: 25,
     poll_interval_average: nil,

lib/sidekiq/cli.rb

@@ -229,7 +229,6 @@ module Sidekiq
 
       # set defaults
       opts[:queues] = ["default"] if opts[:queues].nil?
-      opts[:strict] = true if opts[:strict].nil?
       opts[:concurrency] = Integer(ENV["RAILS_MAX_THREADS"]) if opts[:concurrency].nil? && ENV["RAILS_MAX_THREADS"]
 
       # merge with defaults
@@ -379,6 +378,7 @@ module Sidekiq
 
     def parse_queue(opts, queue, weight = nil)
       opts[:queues] ||= []
+      opts[:strict] = true if opts[:strict].nil?
       raise ArgumentError, "queues: #{queue} cannot be defined twice" if opts[:queues].include?(queue)
       [weight.to_i, 1].max.times { opts[:queues] << queue }
       opts[:strict] = false if weight.to_i > 0

test/config_queues_without_weights.yml

@@ -0,0 +1,4 @@
+---
+:queues:
+  - [queue_1]
+  - [queue_2]

test/test_cli.rb

@@ -238,6 +238,99 @@ describe Sidekiq::CLI do
               assert_equal 7, Sidekiq.options[:queues].count { |q| q == 'often' }
               assert_equal 3, Sidekiq.options[:queues].count { |q| q == 'seldom' }
             end
+
+            describe 'when the config file specifies queues with weights' do
+              describe 'when -q specifies queues without weights' do
+                it 'sets strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config.yml
+                                        -r ./test/fake_env.rb
+                                        -q foo -q bar])
+
+                  assert_equal true, !!Sidekiq.options[:strict]
+                end
+              end
+
+              describe 'when -q specifies no queues' do
+                it 'does not set strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config.yml
+                                        -r ./test/fake_env.rb])
+
+                  assert_equal false, !!Sidekiq.options[:strict]
+                end
+              end
+
+              describe 'when -q specifies queues with weights' do
+                it 'does not set strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config.yml
+                                        -r ./test/fake_env.rb
+                                        -q foo,2 -q bar,3])
+
+                  assert_equal false, !!Sidekiq.options[:strict]
+                end
+              end
+            end
+
+            describe 'when the config file specifies queues without weights' do
+              describe 'when -q specifies queues without weights' do
+                it 'sets strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config_queues_without_weights.yml
+                                        -r ./test/fake_env.rb
+                                        -q foo -q bar])
+
+                  assert_equal true, !!Sidekiq.options[:strict]
+                end
+              end
+
+              describe 'when -q specifies no queues' do
+                it 'sets strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config_queues_without_weights.yml
+                                        -r ./test/fake_env.rb])
+
+                  assert_equal true, !!Sidekiq.options[:strict]
+                end
+              end
+
+              describe 'when -q specifies queues with weights' do
+                it 'does not set strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config_queues_without_weights.yml
+                                        -r ./test/fake_env.rb
+                                        -q foo,2 -q bar,3])
+
+                  assert_equal false, !!Sidekiq.options[:strict]
+                end
+              end
+            end
+
+            describe 'when the config file specifies no queues' do
+              describe 'when -q specifies queues without weights' do
+                it 'sets strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config_empty.yml
+                                        -r ./test/fake_env.rb
+                                        -q foo -q bar])
+
+                  assert_equal true, !!Sidekiq.options[:strict]
+                end
+              end
+
+              describe 'when -q specifies no queues' do
+                it 'sets strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config_empty.yml
+                                        -r ./test/fake_env.rb])
+
+                  assert_equal true, !!Sidekiq.options[:strict]
+                end
+              end
+
+              describe 'when -q specifies queues with weights' do
+                it 'does not set strictly ordered queues' do
+                  subject.parse(%w[sidekiq -C ./test/config_empty.yml
+                                        -r ./test/fake_env.rb
+                                        -q foo,2 -q bar,3])
+
+                  assert_equal false, !!Sidekiq.options[:strict]
+                end
+              end
+            end
           end
 
           describe 'default config file' do