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
Action View Test Case rendered
memoization
#51093
Action View Test Case rendered
memoization
#51093
Conversation
c8a39b9
to
de23549
Compare
Follow-up to [49856][] Follow-up to [49194][] The introduction of memoization as an optimization posed a backwards incompatible change to View tests that call `render` multiple times. This commit changes the `@rendered` instance variable from a `String` to an instance of the `RenderedViewContent` specialized `String` subclass. The end result is that there is no memoization to reset, and the memoization optimization side-effect is preserved after rendering for test cases where `rendered` (or parser methods like `rendered.html`) might be invoked more than once. [49856]: rails#49856 (comment) [49194]: https://github.com/rails/rails/pull/49194/files#diff-ce84a807f3491121a5230d37bd40454bb1407fcca71179e1a2fa76d4c0ddfa2aR293
de23549
to
520cd8f
Compare
…ation Action View Test Case `rendered` memoization
Thank you Sean! Backported to 7-1-stable in ddb959b. |
@@ -283,30 +283,6 @@ const fileInputSelector = Rails.fileInputSelector | |||
Rails.fileInputSelector(...) | |||
``` | |||
|
|||
### `ActionView::TestCase#rendered` no longer returns a `String` |
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.
Why was this removed?
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.
@rafaelfranca https://github.com/rails/rails/pull/49856/files#diff-ce84a807f3491121a5230d37bd40454bb1407fcca71179e1a2fa76d4c0ddfa2aR302 changed that class inheritance hierarchy by making it inherit from String
. That was back ported to 7-1-stable
(in f7255ec, maybe? That commit's contents differ from its title line).
With that change in class hierarchy, this guidance became partly incorrect, since "ActionView::TestCase#rendered
no longer returns a String
" was no longer true. With the change in this commit from a method to an attr_reader
, that guidance became entirely incorrect, since it's both a String
and an attr_reader
.
Motivation / Background
The introduction of memoization as an optimization posed a backwards incompatible change to View tests that call
render
multiple times.Detail
Follow-up to 49856
Follow-up to 49194
This commit changes the
@rendered
instance variable from aString
to an instance of theRenderedViewContent
specializedString
subclass.The end result is that there is no memoization to reset, and the memoization optimization side-effect is preserved after rendering for test cases where
rendered
(or parser methods likerendered.html
) might be invoked more than once.Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]