Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

REBASED: fixing assert_template bug when template matches expected, but not ends with #7659

Merged
merged 1 commit into from

3 participants

@HugoLnx

Pull request #5767 rebased ;)

@drogus
Collaborator
@rafaelfranca

And don't forget to squash your commits.

@drogus
Collaborator

@rafaelfranca since one of the commits is from the other commiter, I would say that squashing commits is not that necassery, in case of revert or something, we can revert merge commit

@rafaelfranca

Fine then

@rafaelfranca

@HugoLnx ping

@HugoLnx

...pong!
I will do the changes next sunday. ;)

@rafaelfranca

Ok. Thanks. :+1:

@HugoLnx HugoLnx `assert_template` no more passing with what ever string that matches.
Given Im rendering an template `/layout/hello.html.erb`, assert_template was
passing with any string that matches. This behavior allowed false passing like:

	assert_template "layout"
	assert_template "out/hello"

Now the passing possibilities are:

	assert_template "layout/hello"
	assert_template "hello"

fixing assert_template bug when template matches expected, but not ends with

Cherry Pick Merge: Fixes issue #3849 assert_template false positive

taking redundant test off

prevening incorrect assert_template when rendering with repeated names in path

updating CHANGELOG with bugfix: assert_template false passing
19dff78
@HugoLnx

Squashed, changeloged and ready! :D

@rafaelfranca rafaelfranca merged commit d0d02bf into from
@rafaelfranca rafaelfranca referenced this pull request from a commit
@rafaelfranca rafaelfranca Revert "Merge pull request #7659 from HugoLnx/template_error_no_match…
…es_rebased"

This reverts commit 7d17cd2.

Conflicts:
	actionpack/CHANGELOG.md

Reason: This added a regression since people were relying on this buggy behavior.
This will introduce back #3849 but we will be backward compatible in
stable release.

Fixes #8068.
d5b275d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 30, 2012
  1. @HugoLnx

    `assert_template` no more passing with what ever string that matches.

    HugoLnx authored
    Given Im rendering an template `/layout/hello.html.erb`, assert_template was
    passing with any string that matches. This behavior allowed false passing like:
    
    	assert_template "layout"
    	assert_template "out/hello"
    
    Now the passing possibilities are:
    
    	assert_template "layout/hello"
    	assert_template "hello"
    
    fixing assert_template bug when template matches expected, but not ends with
    
    Cherry Pick Merge: Fixes issue #3849 assert_template false positive
    
    taking redundant test off
    
    prevening incorrect assert_template when rendering with repeated names in path
    
    updating CHANGELOG with bugfix: assert_template false passing
This page is out of date. Refresh to see the latest.
View
16 actionpack/CHANGELOG.md
@@ -1,5 +1,21 @@
## Rails 4.0.0 (unreleased) ##
+* `assert_template` no more passing with what ever string that matches.
+
+ Given Im rendering an template `/layout/hello.html.erb`, assert_template was
+ passing with any string that matches. This behavior allowed false passing like:
+
+ assert_template "layout"
+ assert_template "out/hello"
+
+ Now the passing possibilities are:
+
+ assert_template "layout/hello"
+ assert_template "hello"
+
+ *Hugolnx*
+
+
* `image_tag` will set the same width and height for image if numerical value
passed to `size` option.
View
17 actionpack/lib/action_controller/test_case.rb
@@ -86,16 +86,23 @@ def assert_template(options = {}, message = nil)
response.body
case options
- when NilClass, String, Symbol, Regexp
+ when NilClass, Regexp, String, Symbol
options = options.to_s if Symbol === options
rendered = @templates
msg = message || sprintf("expecting <%s> but rendering with <%s>",
options.inspect, rendered.keys)
- matches_template =
- if options
+ matches_template =
+ case options
+ when String
+ rendered.any? do |t, num|
+ options_splited = options.split(File::SEPARATOR)
+ t_splited = t.split(File::SEPARATOR)
+ t_splited.last(options_splited.size) == options_splited
+ end
+ when Regexp
rendered.any? { |t,num| t.match(options) }
- else
- @templates.blank?
+ when NilClass
+ rendered.blank?
end
assert matches_template, msg
when Hash
View
22 actionpack/test/controller/action_pack_assertions_test.rb
@@ -7,6 +7,7 @@ class ActionPackAssertionsController < ActionController::Base
def nothing() head :ok end
def hello_world() render :template => "test/hello_world"; end
+ def hello_repeating_in_path() render :template => "test/hello/hello"; end
def hello_xml_world() render :template => "test/hello_xml_world"; end
@@ -464,6 +465,20 @@ def test_fails_with_incorrect_string
end
end
+ def test_fails_with_incorrect_string_that_matches
+ get :hello_world
+ assert_raise(ActiveSupport::TestCase::Assertion) do
+ assert_template 'est/he'
+ end
+ end
+
+ def test_fails_with_repeated_name_in_path
+ get :hello_repeating_in_path
+ assert_raise(ActiveSupport::TestCase::Assertion) do
+ assert_template 'test/hello'
+ end
+ end
+
def test_fails_with_incorrect_symbol
get :hello_world
assert_raise(ActiveSupport::TestCase::Assertion) do
@@ -471,6 +486,13 @@ def test_fails_with_incorrect_symbol
end
end
+ def test_fails_with_incorrect_symbol_that_matches
+ get :hello_world
+ assert_raise(ActiveSupport::TestCase::Assertion) do
+ assert_template :"est/he"
+ end
+ end
+
def test_fails_with_wrong_layout
get :render_with_layout
assert_raise(ActiveSupport::TestCase::Assertion) do
View
1  actionpack/test/fixtures/test/hello/hello.erb
@@ -0,0 +1 @@
+Hello world!
Something went wrong with that request. Please try again.