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

Response sequence #35

Merged
merged 2 commits into from
Dec 21, 2015
Merged

Response sequence #35

merged 2 commits into from
Dec 21, 2015

Conversation

Nemo157
Copy link
Contributor

@Nemo157 Nemo157 commented Dec 18, 2015

According to the VCR docs identical requests in a cassette should be replayed in sequence, this can be useful for writing tests for behaviour such as retrying after receiving a 500 response.

I don't think this is the best implementation, but it's a starting point for discussion (and allows me to start relying on this behaviour).

Also, I'm sure there must be a much nicer way to do Enum.reverse(acc) ++ tail ++ [response], but I'm pretty much a complete beginner to writing Elixir.

According to the VCR docs[1] identical requests in a cassette should be
replayed in sequence, this can be useful for writing tests for behaviour
such as retrying after receiving a 500 response.

[1]: https://relishapp.com/vcr/vcr/v/2-9-3/docs/request-matching/identical-requests-are-replayed-in-sequence
There are definitely much nicer ways to implement this, but as a brute
force starting point it works.
@parroty
Copy link
Owner

parroty commented Dec 20, 2015

Thanks for the PR. The link to the VCR docs you mentioned seems pointing different location (could you verify?).

I think the ordering scenario makes sense. I'm wondering whether it should be default or an optional parameter (safer to make it optional?).

@Nemo157
Copy link
Contributor Author

Nemo157 commented Dec 20, 2015

Heh, seems you can't use numbers as url references in GitHub flavoured markdown. Correct link is https://relishapp.com/vcr/vcr/v/2-9-3/docs/request-matching/identical-requests-are-replayed-in-sequence

@Nemo157
Copy link
Contributor Author

Nemo157 commented Dec 20, 2015

The current behaviour is to ignore all but the first response in the cassette that matches a request. I guess if someone had extra responses that weren't used then it could cause issues if it was on by default.

Maybe it could start as an optional parameter then get switched on by default in some future breaking version?

@parroty
Copy link
Owner

parroty commented Dec 20, 2015

Thanks for the comment.

Maybe it could start as an optional parameter then get switched on by default in some future breaking version?

I was thinking it may be safer approach, but

The current behaviour is to ignore all but the first response in the cassette that matches a request. I guess if someone had extra responses that weren't used then it could cause issues if it was on by default.

This part sounds like it's just a incorrect behavior (I couldn't come up with good option name to explain the behavior difference), and also re-recording would resolve the possible issue.

With the above, I'll be merging and release as v0.7.0 (with some description).
Just confirmation, but does it make sense?

@Nemo157
Copy link
Contributor Author

Nemo157 commented Dec 21, 2015

Sounds good, thanks :)

parroty added a commit that referenced this pull request Dec 21, 2015
@parroty parroty merged commit 86606ae into parroty:master Dec 21, 2015
@parroty
Copy link
Owner

parroty commented Dec 21, 2015

Thanks!

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.

2 participants