Skip to content
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

Should we allow lower-case header names in io.pedestal.service.test/test-servlet-request ? #79

Closed
daemianmack opened this issue May 30, 2013 · 3 comments
Labels

Comments

@daemianmack
Copy link
Member

Re: https://github.com/pedestal/pedestal/blob/master/service/src/io/pedestal/service/test.clj#L96-97

I spent a bit of time trying to pass a "content-type" header to io.pedestal.service.test/response-for before realizing I needed to pass a "Content-Type" header instead.

Do we suppose this is a sufficiently-common error that it's worth searching for arbitrarily-cased headers in the test-servlet-request fn?

If so, I'll submit a PR.

@avescodes
Copy link

Hmm, so we'd be lenient consider things like "content-type" or :content-type or :Content-Type. I wonder how far sweeping this leniency would have to be given we normally deal in maps?

@timewald
Copy link
Contributor

The goal of the faux servlet test infrastructure is to ensure that your
service code works with the data it would get from a servlet, so the goal
should be to emulate that data. That said, the servlet-interceptor
lower-cases header names when it builds the headers map for the request
map. So this seems like a reasonable change in response-for. The main risk
to this change is that if, at some point, servlet-interceptor did something
more with headers, some tests that used lower-case headers might break. I
would prefer we use correct casing for HTTP headers, as per the spec, but
if this is a real pain point, I'm not against making the test jig a little
more lenient.

Tim-

On Fri, May 31, 2013 at 11:41 AM, Ryan Neufeld notifications@github.comwrote:

Hmm, so we'd be lenient consider things like "content-type" or
:content-type or :Content-Type. I wonder how far sweeping this leniency
would have to be given we normally deal in maps?


Reply to this email directly or view it on GitHubhttps://github.com//issues/79#issuecomment-18752926
.

@avescodes
Copy link

It sounds like the answer was a round-about "no."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants