Skip to content

with_priority matcher for ActiveJob #2759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Enhancements:
(Steve Polito, #2746)
* Verify ActiveJob arguments by comparing to the method signature. (Oli Peate, #2745)
* Add suggestion to rails_helper.rb to skip when not in test most. (Glauco Custódio, #2751)
* Add `at_priority` qualifier to `have_enqueued_job` set of matchers. (mbajur, #2759)

Bug Fixes:

Expand Down
19 changes: 19 additions & 0 deletions features/job_specs/job_spec.feature
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ Feature: Job specs
When I run `rspec spec/jobs/upload_backups_job_spec.rb`
Then the example should pass

Scenario: Specify that job was enqueued with a priority
Given a file named "spec/jobs/upload_backups_job_spec.rb" with:
"""ruby
require "rails_helper"

RSpec.describe UploadBackupsJob, type: :job do
describe "#perform_later" do
it "uploads a backup" do
ActiveJob::Base.queue_adapter = :test
expect {
UploadBackupsJob.set(priority: 5).perform_later
}.to have_enqueued_job.at_priority(5)
end
end
end
"""
When I run `rspec spec/jobs/upload_backups_job_spec.rb`
Then the example should pass

Scenario: Specify that job was enqueued with alias block syntax
Given a file named "spec/jobs/upload_backups_job_spec.rb" with:
"""ruby
Expand Down
30 changes: 29 additions & 1 deletion lib/rspec/rails/matchers/active_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class Base < RSpec::Rails::Matchers::BaseMatcher
def initialize
@args = []
@queue = nil
@priority = nil
@at = nil
@block = proc { }
set_expected_number(:exactly, 1)
Expand All @@ -30,6 +31,11 @@ def on_queue(queue)
self
end

def at_priority(priority)
@priority = priority.to_i
self
end

def at(time_or_date)
case time_or_date
when Time then @at = Time.at(time_or_date.to_f)
Expand Down Expand Up @@ -103,7 +109,7 @@ def supports_block_expectations?

def check(jobs)
@matching_jobs, @unmatching_jobs = jobs.partition do |job|
if job_match?(job) && arguments_match?(job) && queue_match?(job) && at_match?(job)
if job_match?(job) && arguments_match?(job) && queue_match?(job) && at_match?(job) && priority_match?(job)
args = deserialize_arguments(job)
@block.call(*args)
true
Expand All @@ -117,6 +123,10 @@ def check(jobs)
return false
end

check_countable
end

def check_countable
@matching_jobs_count = @matching_jobs.size

case @expectation_type
Expand All @@ -131,6 +141,7 @@ def base_message
msg << " with #{@args}," if @args.any?
msg << " on queue #{@queue}," if @queue
msg << " at #{@at.inspect}," if @at
msg << " with priority #{@priority}," if @priority
msg << " but #{self.class::MESSAGE_EXPECTATION_ACTION} #{@matching_jobs_count}"
end
end
Expand All @@ -140,6 +151,12 @@ def base_job_message(job)
msg_parts << "with #{deserialize_arguments(job)}" if job[:args].any?
msg_parts << "on queue #{job[:queue]}" if job[:queue]
msg_parts << "at #{Time.at(job[:at])}" if job[:at]
msg_parts <<
if fetch_priority(job)
"with priority #{fetch_priority(job)}"
else
"with no priority specified"
end

"#{job[:job].name} job".tap do |msg|
msg << " #{msg_parts.join(', ')}" if msg_parts.any?
Expand All @@ -150,6 +167,11 @@ def job_match?(job)
@job ? @job == job[:job] : true
end

# Rails 6.1 serializes the priority with a string key
def fetch_priority(job)
job[:priority] || job['priority']
end

def arguments_match?(job)
if @args.any?
args = serialize_and_deserialize_arguments(@args)
Expand Down Expand Up @@ -187,6 +209,12 @@ def queue_match?(job)
@queue == job[:queue]
end

def priority_match?(job)
return true unless @priority

@priority == fetch_priority(job)
end

def at_match?(job)
return true unless @at
return job[:at].nil? if @at == :no_wait
Expand Down
46 changes: 40 additions & 6 deletions spec/rspec/rails/matchers/active_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,40 @@ def perform; raise StandardError; end
}.to have_enqueued_job.on_queue(:low)
end

it "passes with provided priority number as integer" do
expect {
hello_job.set(priority: 2).perform_later
}.to have_enqueued_job.at_priority(2)
end

it "passes with provided priority number as string" do
expect {
hello_job.set(priority: 2).perform_later
}.to have_enqueued_job.at_priority("2")
end

it "fails when the priority wan't set" do
expect {
expect {
hello_job.perform_later
}.to have_enqueued_job.at_priority(2)
}.to fail_with(/expected to enqueue exactly 1 jobs, with priority 2, but enqueued 0.+ with no priority specified/m)
end

it "fails when the priority was set to a different value" do
expect {
expect {
hello_job.set(priority: 1).perform_later
}.to have_enqueued_job.at_priority(2)
}.to fail_with(/expected to enqueue exactly 1 jobs, with priority 2, but enqueued 0.+ with priority 1/m)
end

pending "accepts matchers as arguments to at_priority" do
expect {
hello_job.set(priority: 1).perform_later
}.to have_enqueued_job.at_priority(eq(1))
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a fee more cases? I can think of a failure case when we expect priority and it wasn’t set. Also when it was set, but doesn’t match the expectation. In both examples with checks if our failure message is reasonable.
I had another scenario on the tip of my tongue, but it keeps slipping. Can you think of other cases we should cover? You can use other similar qualifiers as a source of inspiration .

By the way, does this apply to mailers?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest looks good, thank you!

it "passes with provided at date" do
date = Date.tomorrow.noon
expect {
Expand Down Expand Up @@ -370,14 +404,14 @@ def perform; raise StandardError; end

it "generates failure message with all provided options" do
date = Date.tomorrow.noon
message = "expected to enqueue exactly 2 jobs, with [42], on queue low, at #{date}, but enqueued 0" \
message = "expected to enqueue exactly 2 jobs, with [42], on queue low, at #{date}, with priority 5, but enqueued 0" \
"\nQueued jobs:" \
"\n HelloJob job with [1], on queue default"
"\n HelloJob job with [1], on queue default, with no priority specified"

expect {
expect {
hello_job.perform_later(1)
}.to have_enqueued_job(hello_job).with(42).on_queue("low").at(date).exactly(2).times
}.to have_enqueued_job(hello_job).with(42).on_queue("low").at(date).at_priority(5).exactly(2).times
}.to fail_with(message)
end

Expand Down Expand Up @@ -687,14 +721,14 @@ def perform; raise StandardError; end

it "generates failure message with all provided options" do
date = Date.tomorrow.noon
message = "expected to perform exactly 2 jobs, with [42], on queue low, at #{date}, but performed 0" \
message = "expected to perform exactly 2 jobs, with [42], on queue low, at #{date}, with priority 5, but performed 0" \
"\nQueued jobs:" \
"\n HelloJob job with [1], on queue default"
"\n HelloJob job with [1], on queue default, with no priority specified"

expect {
expect {
hello_job.perform_later(1)
}.to have_performed_job(hello_job).with(42).on_queue("low").at(date).exactly(2).times
}.to have_performed_job(hello_job).with(42).on_queue("low").at(date).at_priority(5).exactly(2).times
}.to fail_with(message)
end

Expand Down