This project is archived and is in readonly mode.

#3260 ✓resolved
Eloy Duran

ActionView::TestCase needs to be ported to master.

Reported by Eloy Duran | September 26th, 2009 @ 12:25 PM | in 3.0.2

I was gonna do this myself in the near future, and still will if there's no interest, but I realized some people might want to work on this as part of the bugmash.

This new implementation of ActionView::TestCase should be ported to master. Should be a good way to get familiar with some of the view internals as well.

http://github.com/rails/rails/commit/cddd4746f9a2fba8215d53a0f9ac52...

Comments and changes to this ticket

  • Elad Meidar

    Elad Meidar September 26th, 2009 @ 07:25 PM

    I've attached a patch that ports ActiveView:TestCase to master

  • Erik Ostrom

    Erik Ostrom September 26th, 2009 @ 09:12 PM

    Patch doesn't apply cleanly to master. There are a couple of simple class renames in the way, and then a more complex refactoring. Several of the tests open ActionController::Helpers#master_helper_module to add helper methods. But that class has moved to AbstractController:Helpers, the method has changed to _helpers, and now it feels like there's an encapsulation boundary there, and opening it up is the wrong way to go about it.

    Working on a revised patch.

  • Erik Ostrom

    Erik Ostrom September 27th, 2009 @ 02:26 AM

    Never mind - it's over my head. Attached is a revision of Elad's patch that got me past the "tests don't run at all" phase and into the "dozens of test failures" phase. This might or might not be better than starting from Elad's patch, or from the original commit.

  • Blue Box Stephen

    Blue Box Stephen September 27th, 2009 @ 07:11 AM

    -1, confirming Erik's assessment that Elad's patch doesn't seem to work against master. And after several hours of basically trying to duplicate what Erik was doing, I think this is also beyond me.

  • Eloy Duran

    Eloy Duran September 27th, 2009 @ 12:49 PM

    Thanks a lot for trying though guys! I'll do this myself then when I find the time.

  • Erik Ostrom

    Erik Ostrom September 27th, 2009 @ 11:14 PM

    Much closer. I expect to have a working patch later today.

  • Erik Ostrom

    Erik Ostrom September 28th, 2009 @ 07:54 AM

    I've attached a patch that makes all the tests pass. I haven't actually verified that you can use it in a real app. But I'm optimistic!

    I am somewhat skeptical about the code around test helpers. It breaks the abstraction boundary around AbstractController::Helpers. It would be better if it could use the Helpers API (or if the Helpers API could be extended for that purpose).

  • Eloy Duran

    Eloy Duran September 28th, 2009 @ 09:54 AM

    It looks good, I'll try it out tonight.

    I am somewhat skeptical about the code around test helpers. It breaks the abstraction boundary around AbstractController::Helpers. It would be better if it could use the Helpers API (or if the Helpers API could be extended for that purpose).

    This code breaks the abstraction boundary around AbstractController::Helpers, and in fact includes tests to verify that breaking the abstraction boundary is effective.

    Could you elaborate on this? For instance, in the which tests is “breakage” tested?

    And I assume you're talking about the ActionController::Helpers/AbstractController::Helpers API? That could certainly be refactored yes, for instance, the helpers shouldn't use “controller” imo to call helper methods on. Otherwise, like in this example, it has to be monkeypatched. So one way would be if Helpers.helper_method, accepts both the method to make available, plus the receiver, instead of assuming “controller” as the receiver. The controller class can then simply override it and super with self as the receiver.

  • Erik Ostrom

    Erik Ostrom September 28th, 2009 @ 06:11 PM

    Yeah, that extension of helper_method is just what I had in mind.

    I was confused when I wrote about "tests to verify". I was referring to "is able to make methods available to the view" and "named routes can be used from helper included in view", which both open _helpers (formerly master_helper_module). But they do it as a way to set up the test - not as the behavior to be tested. Sorry I got that wrong.

  • Eloy Duran

    Eloy Duran September 28th, 2009 @ 07:08 PM

    • State changed from “new” to “verified”
    • Assigned user changed from “Eloy Duran” to “josh”

    I created an application with a helper, and some simple tests, routes, rendering with ivars and lvars etc. It all worked fine, thanks! So +1 and assigning to Josh. :)

    I just had to add one extra require statement to the test case file. This problem was only encountered when running from an app instead of running the actionpack test suite.

    I created a new patch with the extra require and adjusted the commit message. All ok with you?

  • Eloy Duran

    Eloy Duran September 28th, 2009 @ 07:11 PM

    On the topic of the Helpers API etc. Today I thought it would be better to forward almost all method calls to the TestController instance, this would make params, session, request etc work a expected. So these current "hacks" are then probably no longer needed, the only hack that would be needed is to override render so it sets the ivars on the controller and then forwards the call to controller.render which will set the ivars on the view instance.

    Still, if you feel like cleaning what we discussed earlier up right now, feel free of course :)

  • Eloy Duran
  • Repository

    Repository September 28th, 2009 @ 07:31 PM

    • State changed from “verified” to “resolved”

    (from [8ffc2e3b8de38c485fa24820208b40920dad7ae3]) Ported the new ActionView::TestCase from 2-3-stable to master [#3260 state:resolved]

    The test case now mimicks the template environment more closely, so it's
    possible to use render, load helper dependencies.

    This also fixes assert_select, and similar assertions. Because view tests
    and helpers generally don't render full templates assert_select looks
    first in rendered and then in output_buffer to find the rendered output.

    Additional master'-only changes: Made the Action Pack Rakefile run the<br/> ActionView::TestCase tests, and made ActionView::Rendering#_render_text<br/> always return a string.

    Signed-off-by: Joshua Peek josh@joshpeek.com
    http://github.com/rails/rails/commit/8ffc2e3b8de38c485fa24820208b40...

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
    • Importance changed from “” to “”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages