-
Notifications
You must be signed in to change notification settings - Fork 21
[WAIT] Issue 14: Add Request to ResponsePredicate #36
Conversation
@jkarni I think this one's ready, I just need a bit of help thinking through any broader implications (CHANGELOG, etc.). |
@@ -429,7 +430,8 @@ finishPredicates p req mgr = go `catch` \(e :: PredicateFailure) -> return $ Jus | |||
where | |||
go = do | |||
resps <- getRequestPredicate (requestPredicates p) req mgr | |||
mapM_ (getResponsePredicate $ responsePredicates p) resps | |||
let responder = getResponsePredicate (responsePredicates p) req |
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.
Hmm, this isn't quite right, I think. The actual requests that resulted in the responses resps
are not necessarily req
. Rather, they are the requests generated by the RequestPredicate
s based on req
.
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.
I have to think a little more about this. It feels like it's becoming evident that the Request
/ResponsePredicate
distinction is somewhat artificial or misleading. Really what's at stake is something like the distinction between request generator (or generator modifiers) and (request + response) predicates. But separating those two too much is also wrong, since usually one wants to generate certain requests in order to check the responses.
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.
I am glad to have your perspective on this. I was wondering about the difference between ResponsePredicate
and RequestPredicate
when I was working on this PR, but I figured the distinction is still valid as a RequestPredicate
needs a Manager
in order to actually issue requests.
I have to confess I didn't look closely enough at finishPredicates
, so it's a relief that you caught my mistake, which I can see clearly now.
I agree with what you're saying that checking a particular response is really often about a (Request, Response)
pair. I was kind of errantly thinking about bifunctors or profunctors when looking at this code this weekend, but I didn't follow the thought very far.
I have been thinking about this PR and I think I'd like to withdraw it. I have some thoughts that I'd like to explore a bit before coming up with a suggestion for representing predicates based on I have been thinking about this in light of this comment from the
While I have some ideas on representing these things, this part is kind of stumping me. |
Description
This PR does the following:
Adds the
Request
toResponsePredicate
so thatPredicateFailure
can pretty print the request as well as the failing response.Removes the
Maybe
fromPredicateFailure (Maybe Request)
now that bothRequestFailure
andResponseFailure
require the request.Adds tests for the following predicates:
not500
andunauthorizedContainsWWWAuthenticate
Fixes #14