should_respond_with_content_type does not test against the HTTP header #128

Closed
mike-burns opened this Issue Jul 9, 2012 · 6 comments

Comments

Projects
None yet
5 participants
Owner

mike-burns commented Jul 9, 2012

@jarl-dk reports in thoughtbot/shoulda#97 :

Hi I am converting some existing tests to shoulda.

My current test looks like this:
assert_equal Mime::KML.to_s + '; charset=utf-8', @response.header["Content-Type"]

I replace this with
should_respond_with_content_type Mime::KML.to_s + '; charset=utf-8'

but that fails because shoulda tests against @controller.response.content_type and not @controller.response.header["Content-Type"]

@calebthompson calebthompson added a commit to calebthompson/shoulda-matchers that referenced this issue Jul 11, 2012

@calebthompson calebthompson Initial stabs at fixing #128
Seems that header["Content-Type"] does indeed return a different value (at least
for :xml) than content_type, but the tests all expect that the response will be
that of content_type and not header["Content-Type"].

It also occurs to me that there ought to be tests for content types other than
:xml, just in case.
b267c10

So the problem, as I'm sure you're aware, is that content_type does not contain the same value as header["Content-Type"].

@controller.response.content_type
#=> "application/xml"
@controller.response.header["Content-Type"]
#=> "application/xml; charset=utf-8"

The RespondWithContentTypeMatcher is using Mime::Type lookups when passed a symbol, which returns 'application/xml' for :xml. The respond_with_content_type_matcher_spec is checking against the literal string 'application/xml' as well.

It wouldn't be super difficult to change the RespondWithContentTypeMatcher#response_content_type method to check agains header["Content-Type"], but doing so will break the existing specs and will likely not be a backward-compatible change.

I'd be glad to fix up my branch and open a pull request for this, but my guess is that @jari-dk should just be using

describe BlahController do
  should_respond_with_content_type Mime::KML.to_s
  # or
  should_respond_with_content_type :kml
  ...
end

jarl-dk commented Aug 8, 2012

Thank you @mike-burns for creating the issue in the right project.
Thank you @calebthompson for ellaborating on the design and intended use of shoulda and the matchers.

I have now updated and changed my tests to look like this:

should respond_with_content_type :kml

I have one question though.

The above test will most likely pass even if the response header header["Content-Type"] contains the string 'application/xml; charset=ISO-8859-1'.

What type of matcher should I use to make sure the the part after ';' in header["Content-Type"] is charset=utf-8?

Thanks in advance...

Contributor

gabebw commented Dec 27, 2012

@jarl-dk - you mention that it will "most likely" pass even with a scenario that should be failing (XML instead of KML). Does it actually fail in that case? I'd prefer not to write code for something that isn't broken :)

jarl-dk commented Jan 16, 2013

@gabebw : I have not been clear (I intermixed xml and kml which caused confusion). It is not important that I respond KML.

I would like to ensure (i.e. test) that my Content-Type response header has the value application/vnd.google-earth.kml+xml; charset=utf-8

It seems that a test like

should respond_with_content_type :kml 

also passes if the Content-Type response header has the value application/vnd.google-earth.kml+xml (that is, even without the charset=utf-8 part.) because it tests against @controller.response.content_type which in both cases are application/vnd.google-earth.kml+xml

Member

mxie commented Mar 25, 2013

Closing this since we will be deprecating this matcher in the next major release.

mxie closed this Mar 25, 2013

jarl-dk commented Mar 26, 2013

OK. Are there any replacements? I.e. what is the recommended way to test the value of the response header "Content-Type"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment