Conversation
|
Maybe someone can help with the failing checks, I'm not sure what to do |
|
@23tux This looks good to me. Those checks are failing on |
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
|
@boardfish I added a test helper as well, could you have another look at it? Apart from the integration/system specs that require a browser (as I said, I had some problems with the setup and can't run them), all the tests pass on my machine. I added one commit to make the allocation spec more robust. And the Lint check also fails, but it doesn't seem to involve my changes. |
Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
|
@23tux thank you for taking the time to make this contribution! In https://viewcomponent.org/best_practices.html#test-against-rendered-content-not-instance-methods, we say:
I stand by that recommendation and thus am hesitant to add any test helpers to enable testing component instance methods. There is a reason https://viewcomponent.org/best_practices.html#most-viewcomponent-instance-methods-can-be-private immediately follows the guidance above! I am happy to have my mind changed here, but for now I'm going to close this PR in favor of #2511 which cherry-picks your bug fix ❤️ |
What are you trying to accomplish?
t(".some_key")inside a#render?method. Think ofdef render? = items.any?anditemsis a hash of translations@virtual_pathis actually set, I can now testt(..)calls in my helper method, because the teardown hasn't happened yet.What approach did you choose and why?
#render_inmethod into methods.@view_context.instance_variable_set(:@virtual_path, virtual_path)outside of the@output_buffer.with_buffer doblock. I just hope this doesn't break anything else.Anything you want to highlight for special attention from reviewers?
assert_allocations