Skip to content

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

Merged
merged 1 commit into from
Oct 12, 2016
Merged

Improve failure message for AJ matcher #1722

merged 1 commit into from
Oct 12, 2016

Conversation

morgoth
Copy link
Contributor

@morgoth morgoth commented Oct 11, 2016

Printing unmatched jobs should help in finding failure reason.

If you have some suggestions how to improve this message, let me know.

@@ -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|
Copy link
Member

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?

Copy link
Contributor Author

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:"
Copy link
Member

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:"

Copy link
Contributor Author

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)}"
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and done

@JonRowe JonRowe merged commit b1aac09 into rspec:master Oct 12, 2016
JonRowe added a commit that referenced this pull request Oct 12, 2016
@JonRowe
Copy link
Member

JonRowe commented Oct 12, 2016

Thanks :)

sebjacobs pushed a commit to futurelearn/rspec-rails that referenced this pull request Mar 15, 2019
sebjacobs pushed a commit to futurelearn/rspec-rails that referenced this pull request Mar 15, 2019
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.

2 participants