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

added Test.Hspec.Wai.Server #30

Closed
wants to merge 1 commit into from
Closed

added Test.Hspec.Wai.Server #30

wants to merge 1 commit into from

Conversation

soenkehahn
Copy link
Contributor

This commit adds a helper function that allows to test wai Applications over a real port. This is very different from the approach in Test.Hspec.Wai but I still think this is the best place for this helper function.

@soenkehahn
Copy link
Contributor Author

Strictly speaking this also has nothing to do with hspec, but it plays very nicely with it.

@sol
Copy link
Member

sol commented Dec 13, 2015

@soenkehahn sorry for the delay. I imagine it should be possible to test WAI applications both directly (that is what we currently do) and through HTTP (by starting warp and doing proper HTTP requests, I assume what you are doing) with the same API.

We would change the state of the WaiSession from Application to something like Either Application Port and then either use wai-test or http-client to query. We can then have two primitives

with :: IO Application -> SpecWith (Either Application Port) -> Spec

and

withSocket :: IO Application -> SpecWith (Either Application Port) -> Spec

that pass on the application or start warp and pass on the port respectively.

Does this make sense?

If we go for that approach, then hspec-wai would also be the right place for this code.

Going even further, we could make this even more extensible by using some action of type

Method -> Path -> Headers -> Body -> IO Response

as the state of the session. That way you could test arbitrary HTTP applications, directly or through some server, with the same API. That would also mean that e.g. @dbp could adapt hspc-snap to use the same API.

@sol
Copy link
Member

sol commented Dec 18, 2015

@soenkehahn I created #31 to illustrate what I meant in my previous comment. If you think that is a good idea, still needs some cleanup, documentation, tests, etc...

@soenkehahn
Copy link
Contributor Author

Also, sorry for the delay. Christmas got in the way.

The two use-cases that prompted me to add Test.Hspec.Wai.Server were both more low-level and being able to use e.g. Test.Hspec.Wai.get wouldn't have helped me at all. (One was the test-suite for servant-client where we test the functions that actually access the network to issue requests, so we don't use get, post, etc.. The other was a standalone script that needed to open an arbitrary free port, so I just needed something like openFreePort.) So I don't see a reason (yet) for providing the same API for both test methods. But this again makes me think whether another package would be better suited for containing these functions. Maybe mockery?

Another idea is this: Why don't we remove the direct testing and replace it completely with the HTTP-based testing? Then we provide an additional function like getPort :: WaiSession Port. That would allow to get the port, ignore get, post, etc. and access the server through the network directly. But the normal usage of hspec-wai would be unaffected. (I'm hoping the slowdown of running the tests caused by going through the network would be marginal.)

@sol: wdyt?

@soenkehahn
Copy link
Contributor Author

@sol: Any thoughts on this?

Today I stumbled upon a situation where I need to test an Application that uses requestHeaderHost. hspec-wai doesn't support that, because it creates the wai Request without setting that field. Even setting the Host header through hspec-wai's request doesn't work, since that header won't be looked at to fill in requestHeaderHost. So I'm more and more liking my idea to test Applications through a real port, to be as close to production as possible.

@soenkehahn
Copy link
Contributor Author

On the other hand by now I think that this would ideally go into the warp package.

@snoyberg: Would you be interested in a PR that adds withApplication and openFreePort to warp?

withApplication: https://github.com/hspec/hspec-wai/pull/30/files#diff-ef5e60dd3e66f3ddc7ae63484318cefeR25
openFreePort: https://github.com/hspec/hspec-wai/pull/30/files#diff-ef5e60dd3e66f3ddc7ae63484318cefeR76

@snoyberg
Copy link

No objection in principle to the addition of such functions to warp, though
the implementation may be much cleaner by using the async package.

On Thu, Feb 11, 2016 at 3:01 PM, Sönke Hahn [email protected]
wrote:

On the other hand by now I think that this would ideally go into the warp
package.

@snoyberg https://github.com/snoyberg: Would you be interested in a PR
that adds withApplication and openFreePort to warp?

withApplication:
https://github.com/hspec/hspec-wai/pull/30/files#diff-ef5e60dd3e66f3ddc7ae63484318cefeR25
openFreePort:
https://github.com/hspec/hspec-wai/pull/30/files#diff-ef5e60dd3e66f3ddc7ae63484318cefeR76


Reply to this email directly or view it on GitHub
#30 (comment).

@sol
Copy link
Member

sol commented Feb 19, 2016

@soenkehahn I'm in favor of adding withApplication to warp.

As for hspec-wai, I think giving the user the choice of testing through real HTTP or in-process is the right thing.

@sol
Copy link
Member

sol commented Aug 25, 2021

This is part of warp by now. Closing.

@sol sol closed this Aug 25, 2021
@sol sol deleted the server branch August 25, 2021 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants