feat: support response assertions #76
Conversation
| response = Struct.new(:source).new('response') | ||
| new(expected, response, failure_message.first) |
There was a problem hiding this comment.
note: we're using Struct here because the "actual" for this assertion is the test-local @response property - I think this is a good solution especially as BasicAssertion immediately calls source and stores that value rather than holding any further reference to the actual parameter, though I wouldn't be surprised if there is a Rubocop helper for these situations that we could use instead
There was a problem hiding this comment.
That’s clever 😅 Maybe it’s a sign that we should pass the actual source values for actual and expected instead of the Node instances. But let’s do that in a later PR.
Could I get you to extract the response struct into a constant, so we don’t define it over and over?
| end | ||
| end | ||
|
|
||
| context 'with response assertions' do |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
nzlaura
force-pushed
the
fix/add-assert-response
branch
from
d558a4d to
aed0581
Compare
There was a problem hiding this comment.
Hi @nzlaura and thank you for the pull request 🙏🏼
I added a comment on the code, and have two more requests:
- It looks like you are using Ruby 3.3 or older, which results in the diff on docs/modules/ROOT/pages/cops_rspecrails.adoc (
foo => bar→foo=>bar). CI runs Ruby 3.4, which is why it currently fails. (Yes, pPerhaps we should add a .ruby-version file to the repo.) - Could you squash your commits, please? I think all (currently) 6 commits are part of the same “unit of work” and are best described & reviewed together.
| response = Struct.new(:source).new('response') | ||
| new(expected, response, failure_message.first) |
There was a problem hiding this comment.
That’s clever 😅 Maybe it’s a sign that we should pass the actual source values for actual and expected instead of the Node instances. But let’s do that in a later PR.
Could I get you to extract the response struct into a constant, so we don’t define it over and over?
nzlaura
force-pushed
the
fix/add-assert-response
branch
from
aed0581 to
fc0cfc1
Compare
|
Thanks for the quick review @bquorning 😄 I've shifted the response struct into a constant, changed to using Ruby 3.4 locally and re run the |
4 participants
This PR adding support for correcting response assertions alongside other minitest assertions.
I have used
Structfor theactualparam for this assertion, because the "actual" is the test-local @response property, and the way the minitests are initialised requireactualto be passed as a param.Before submitting the PR make sure the following are checked:
master(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).