search

oliyh / martian

The HTTP abstraction library for Clojure/script, supporting OpenAPI, Swagger, Schema, re-frame and more
MIT License
525 stars 42 forks source link

feat: Update vcr.cljc to use tripod.context instead of martian.interceptors #204

Open lvh opened 3 weeks ago

lvh commented 3 weeks ago

References #181.

remove-stack prevents all further interceptors from running. It uses terminate under the hood. Critical difference: terminate just means we'd move to the second phase (:leave), ensuring things like e.g. response body encoding still happens.

oliyh commented 3 weeks ago

This looks good, I am very grateful for your efforts here. I'm concerned that no tests failed! I would like to understand that first before merging this.

lvh commented 3 weeks ago

Sure! I'm not sure if you saw my comments on #181 already when writing that. My theory for why the tests didn't fail is that the dummy response in the test had an already-parsed body, but a real HTTP client would return a String. Normally, the :leave stage would parse it for you. Here that wasn't happening but nobody noticed because the output didn't need parsing. Hence my semantic confusion comment from #181: I think the test had a false negative because the dummy response is incorrectly being treated as both an HTTP response hot off the client, and an otherwise already cleaned up response-for.

I think a good test would be for the dummy response to return a String but the test to validate that it is no longer a String, hence validating :leave ran.

oliyh commented 3 weeks ago

Yes, that would be a good test to have - thank you for explaining the current behaviour

lvh commented 3 weeks ago

Well, I'm glad I wrote the test because now I'm not sure I'm doing it correctly. I added a commit, but the tests don't pass :|

Could you check me on the test behavior and confirm that it's asking for the correct behavior?