-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Improve failure message for AJ matcher #1722
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
Conversation
@@ -89,7 +96,7 @@ def supports_block_expectations? | |||
private | |||
|
|||
def check(jobs) | |||
@matching_jobs_count = jobs.count do |job| | |||
@matching_jobs, @unmatching_jobs = jobs.partition do |job| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this return jobs from another class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it will return all jobs that were queued during given action.
I think most of the time number of jobs should be low, so we don't need to scope only to the same class jobs.
This way it's clear for end user what's going on, and it may happen that mistake is also in job class name (like copy pasting) ;-)
"expected to enqueue #{base_message}" | ||
"expected to enqueue #{base_message}".tap do |msg| | ||
if @unmatching_jobs.any? | ||
msg << "\nUnmatched enqueued jobs:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would read better as "Queued jobs:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
if @unmatching_jobs.any? | ||
msg << "\nUnmatched enqueued jobs:" | ||
@unmatching_jobs.each do |job| | ||
msg << "\n* #{base_job_message(job)}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use a '*' elsewhere when indenting, maybe for consistency just go with 2 spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and done
Thanks :) |
[skip ci]
Printing unmatched jobs should help in finding failure reason.
If you have some suggestions how to improve this message, let me know.