[ci skip] Do not recommend testing stored object in template #33833
Conversation
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @georgeclaghorn (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
| @@ -1028,7 +1028,6 @@ You should test for things such as: | |||
| * was the web request successful? | |||
| * was the user redirected to the right page? | |||
| * was the user successfully authenticated? | |||
| * was the correct object stored in the response template? | |||
There was a problem hiding this comment.
I think removing this is the wrong answer. assigns checks the instantied controller ivars, but the wording of this is still a little off. Can we change it to * was the correct information displayed in the response??
There was a problem hiding this comment.
Hey @gmcgibbon. Perfect. I think it makes total sense!. I updated the commit with your suggestion. But one question:
Should it be:
* was the correct information displayed in the response?
or maybe
* was the correct information displayed in the view?
?
Because of the next line uses in the view instead of in the response
* was the appropriate message displayed to the user in the view?
What do you think?
thanks 🍻
There was a problem hiding this comment.
After giving this some thought, I like response better because you could be testing a controller that returns JSON, or something else that's less of a view and more of a representation of data. It also seems less redundant to the previous point of was the appropriate message displayed to the user in the view?.
Since `assigns` is not placed by default with Rails, we suggested to test if the right information was displayed instead of suggesting to test if the right object was stored in the template.
duduribeiro
force-pushed
the
patch-2
branch
from
9988bb9
to
919b508
Compare
| @@ -1028,7 +1028,6 @@ You should test for things such as: | |||
| * was the web request successful? | |||
| * was the user redirected to the right page? | |||
| * was the user successfully authenticated? | |||
| * was the correct object stored in the response template? | |||
There was a problem hiding this comment.
After giving this some thought, I like response better because you could be testing a controller that returns JSON, or something else that's less of a view and more of a representation of data. It also seems less redundant to the previous point of was the appropriate message displayed to the user in the view?.
Summary
I don't know if this PR is a valid one.
Since
assignsis not placed by default with Rails, we removed the suggestion to test if the correct object was stored in the response template.