Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

[PATCH] assert_template wrong behavior for testing layout used to render template?

#5247

I am wondering why testing that a specific layout was used to render a template is dependent on you first testing that a partial is being rendered. From the code:

  when Hash
    if expected_partial = options[:partial]
      if expected_locals = options[:locals]
        actual_locals = @locals[expected_partial.to_s.sub(/^_/,'')]
        expected_locals.each_pair do |k,v|
          assert_equal(v, actual_locals[k])
        end
      elsif expected_count = options[:count]
        actual_count = @partials[expected_partial]
        msg = build_message(message,
                "expecting ? to be rendered ? time(s) but rendered ? time(s)",
                 expected_partial, expected_count, actual_count)
        assert(actual_count == expected_count.to_i, msg)
      elsif options.key?(:layout)
        msg = build_message(message,
                "expecting layout <?> but action rendered <?>",
                expected_layout, @layouts.keys)

        case layout = options[:layout]
        when String
          assert(@layouts.include?(expected_layout), msg)
        when Regexp
          assert(@layouts.any? {|l| l =~ layout }, msg)
        when nil
          assert(@layouts.empty?, msg)
        end
      else
        msg = build_message(message,
                "expecting partial <?> but action rendered <?>",
                options[:partial], @partials.keys)
        assert(@partials.include?(expected_partial), msg)
      end
    else
      assert @partials.empty?,
        "Expected no partials to be rendered"
    end
  end

Notice that the test for the "layout" param (elsif options.key?(:layout)) is nested within the check to see if you are testing for partials (if expected_partial = options[:partial]).

If I assume that this is correct behavior, which I don't think it should be, even if I pass in :partial => "something" then I get an error that says "undefined local variable expected_layout" because no where in assert_template does that variable get assigned.

I have looked through master and have found some tests that seem to be testing this but I am not sure that they are working properly because if you don't pass in the :partial option then it ignores the three assertions that test the @layouts collection.

Patches upcoming...

Reported by Mike Gehard · July 30th, 2010 @ 05:53 AM

State: open
Milestone: 3.0.6
Assigned to: Yehuda Katz (wycats) Yehuda Katz (wycats)
Importance: Low

Activity

  1. Mike Gehard
    Mike Gehard

    I have confirmed that assert_template does not work as you would expect and as the tests show in /actionpack/test/controller/layout_test.rb.

    Example:
    class HasOwnLayoutController < LayoutTest

    layout 'item'

    layout 'item123' end

    def test_layout_set_when_set_in_controller

    @controller = HasOwnLayoutController.new
    get :hello
    assert_template :layout => "layouts/item"
    

    end

    This test still passes with the changed layout.

    July 30th, 2010 @ 03:40 PM

  2. Mike Gehard
    Mike Gehard
    • Tag set to patch

    Here is the patch that makes checking for layouts independent of checking for partials.

    August 2nd, 2010 @ 04:01 PM

  3. Mike Gehard
    Mike Gehard
    • Tag changed from patch to activesupport, patch, rails3

    August 10th, 2010 @ 03:35 PM

  4. Jean
    Jean
    • Title changed from assert_template wrong behavior for testing layout used to render template? to [PATCH] assert_template wrong behavior for testing layout used to render template?

    I confirm that I have this problem too.

    Introduced here : http://github.com/rails/rails/commit/947f86c699b33bd44703b3554db58e...

    August 15th, 2010 @ 08:31 PM

  5. Jean
    Jean

    Is there a problem with the patch ?

    It seems to work for me and in the meantime using the old way (@controller.response.layout) raises a deprecation warning which tells us to use template.layout instead. This would be fine except for 2 things :
    - @controller.response.template or @controller.template both return nil (even though @controller.response.layout isn't) - The template method itself is in the deprecated helper and I suppose will be deprecated soon enough

    August 18th, 2010 @ 01:09 PM

  6. Santiago Pastorino
    Santiago Pastorino
    • Milestone cleared.
    • State changed from new to open
    • Assigned user set to José Valim
    • Importance changed from to Low

    This patch does not apply cleanly anymore. Would you mind to do it again, thank you.

    August 24th, 2010 @ 12:25 PM

  7. Jean
    Jean

    I checked out the latest rails 3-0-stable and recreated the patch.

    August 24th, 2010 @ 01:34 PM

  8. Santiago Pastorino
    Santiago Pastorino
    • Assigned user changed from José Valim to Yehuda Katz (wycats)

    August 24th, 2010 @ 06:19 PM

  9. Jeremy Kemper
  10. Jeremy Kemper
  11. Ryan Bigg
    Ryan Bigg
    • Tag cleared.

    Automatic cleanup of spam.

    October 16th, 2010 @ 02:42 AM

  12. Dave Myron
    Dave Myron

    It seems like the patch will no longer catch :layout => false since it does an if expected_layout = options[:layout] - which will never pass if :layout is false. It should use the original method of checking if the :layout key is present.

    October 21st, 2010 @ 11:39 AM

  13. Santiago Pastorino
  14. Santiago Pastorino
  15. Santiago Pastorino
    Santiago Pastorino
    • Milestone changed from 3.0.5 to 3.0.6

    February 27th, 2011 @ 03:15 AM

  16. Geoffrey Hichborn
    Geoffrey Hichborn

    This bug appears to be a symptom of this: http://groups.google.com/group/rubyonrails-talk/browse_thread/threa...

    The bug isn't really assert_template, but the fact that using :partial and :layout together force the layout to be a partial.

    April 7th, 2011 @ 12:15 AM