-
-
Notifications
You must be signed in to change notification settings - Fork 1k
modify error message when redirected but expecting to render a template #1440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,18 +9,37 @@ def initialize(scope, expected, message = nil) | |
@expected = Symbol === expected ? expected.to_s : expected | ||
@message = message | ||
@scope = scope | ||
@redirect_is = nil | ||
end | ||
|
||
# @api private | ||
def matches?(*) | ||
match_unless_raises ActiveSupport::TestCase::Assertion do | ||
match_check = match_unless_raises ActiveSupport::TestCase::Assertion do | ||
@scope.assert_template expected, @message | ||
end | ||
check_redirect unless match_check | ||
match_check | ||
end | ||
|
||
# Uses normalize_argument_to_redirection to find and format | ||
# the redirect location. normalize_argument_to_redirection is private | ||
# in ActionDispatch::Assertions::ResponseAssertions so we call it | ||
# here using #send. This will keep the error message format consistent | ||
# @api private | ||
def check_redirect | ||
response = @scope.response | ||
return unless response.respond_to?(:redirect?) && response.redirect? | ||
@redirect_is = @scope.send(:normalize_argument_to_redirection, response.location) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the YARD doc is added, can you include some details about this |
||
|
||
# @api private | ||
def failure_message | ||
rescued_exception.message | ||
if @redirect_is | ||
rescued_exception.message[/.* but /] + | ||
"was a redirect to <#{@redirect_is}>" | ||
else | ||
rescued_exception.message | ||
end | ||
end | ||
|
||
# @api private | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,6 +88,24 @@ def assert_template(*); raise "oops"; end | |
end.to raise_exception("oops") | ||
end | ||
end | ||
|
||
context "when fails with a redirect" do | ||
let(:response) { ActionController::TestResponse.new(302) } | ||
def assert_template(*) | ||
message = "expecting <'template_name'> but rendering with <[]>" | ||
raise ActiveSupport::TestCase::Assertion.new(message) | ||
end | ||
def normalize_argument_to_redirection(response_redirect_location) | ||
"http://test.host/widgets/1" | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of |
||
it "gives informative error message" do | ||
response = ActionController::TestResponse.new(302) | ||
response.location = "http://test.host/widgets/1" | ||
expect do | ||
expect(response).to send(template_expectation, "template_name") | ||
end.to raise_exception("expecting <'template_name'> but was a redirect to <http://test.host/widgets/1>") | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤️ yay for specs! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normally we'd have spacing around those methods and the it too, feel like cleaning it up @cupakromer ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤️ |
||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a YARD doc comment.