Skip to content
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

with_priority matcher for ActiveJob #2759

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mbajur
Copy link

@mbajur mbajur commented Apr 18, 2024

See #2757

hello_job.set(priority: 2).perform_later
}.to have_enqueued_job.with_priority("2")
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!

@pirj
Copy link
Member

pirj commented Apr 18, 2024

It also makes sense to add/extend scenario here

Given a file named "spec/jobs/upload_backups_job_spec.rb" with:

Those changes will appear here https://rspec.info/features/6-1/rspec-rails/job-specs/job-spec/, and some prefer those docs to the rdoc ones.

@@ -30,6 +31,11 @@ def on_queue(queue)
self
end

def with_priority(priority)
Copy link
Member

Choose a reason for hiding this comment

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

Personally I would call this priority or at_priority to disambigute from with

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants