Skip to content

Conversation

@23tux
Copy link

@23tux 23tux commented Oct 28, 2025

What are you trying to accomplish?

  1. Sometimes we call t(".some_key") inside a #render? method. Think of def render? = items.any? and items is a hash of translations
  2. To test a components method, I'd like to be able to just do the setup of a component's render lifecycle inside my test. This shortens our test run time, as we don't need to actually render the HTML. And due to the change when the @virtual_path is actually set, I can now test t(..) calls in my helper method, because the teardown hasn't happened yet.

What approach did you choose and why?

  • I extracted the setup, actual render, and teardown of the #render_in method into methods.
  • I moved the line @view_context.instance_variable_set(:@virtual_path, virtual_path) outside of the @output_buffer.with_buffer do block. I just hope this doesn't break anything else.

Anything you want to highlight for special attention from reviewers?

  • I couldn't get the system tests with a real browser to run, but all the other tests are green
  • I'm not sure if I can just change the values in assert_allocations

@23tux
Copy link
Author

23tux commented Oct 28, 2025

Maybe someone can help with the failing checks, I'm not sure what to do

@boardfish
Copy link
Collaborator

@23tux This looks good to me. Those checks are failing on main – I don't think they're blockers to merging this, but because this affects the number of allocations it'd probably be good to make sure those tests pass locally if it's convenient to do so.

@23tux
Copy link
Author

23tux commented Oct 28, 2025

@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 <[email protected]>
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.

3 participants