Recording new episodes with existing cassettes.

[thegreatape]

Hello! My team has been using vcr-clj to test a number of things and it's often a little laborious for us to re-record cassettes entire when new calls need to be added. So, I added a new flag to with-cassette that allows new calls to be added to existing cassettes when set. Let me know what you think!

[gfredericks]

Just so I understand the use case, when you said re-recording is "laborious" is that because you have a single test with many recorded calls that is very slow to record, but recording one extra call is fast? or is it about having to go find the right cassette to delete before running?

[thegreatape]

In our case, we're recording calls against third party apis- the structure and semantics of those responses are stable, but the data contained in them will often shift over time. The laborious part for us, when adding a new API call to our system, is having to delete the entire cassette and then adjust and re-verify our tests' expectations for the old calls agains the newly recorded data.

[gfredericks]

Is it not practical to constrain the assertions to the part of the data that doesn't change over time?

In all the usages of vcr-like libs that I've seen, the main point is repeatability and performance; if the test suite fails when re-recording, that indicates a problem (either a bug in the code, that was coincidentally not exposed by the old data, or an assumption made by the code about the external service that held in the past but has since changed).

It also feels weird to end up with a cassette where different parts were recorded at different times.

I'm not sold against it, I just want to make sure it's valuable enough to justify the added code and API complexity.

[thegreatape]

Is it not practical to constrain the assertions to the part of the data that doesn't change over time?

Not really in our case. Imagine a service that integrates with an API providing weather or Twitter data; it might be important to capture things like today's forecast or current follower counts (and assert that you've captured them accurately in your test suite), but you'd reasonably expect them to change over time.

It's a tradeoff for sure, and a tricky part of testing interactions with third-party APIs—but it's an approach I've used before (using Ruby VCR's new_episodes mode) and I think it's a tradeoff worth making.

[gfredericks]

If it were me I would probably separate the tests into two categories -- one using vcr that checks that the code can get a real response and do something general with it, and another category without vcr of unit tests with specific input/output examples that aren't necessarily as rich as a real interaction.

But I don't think this library needs to have such specific opinions. Knowing that the ruby vcr has a feature like that is very helpful, thanks for the link.

I will look at the code more closely this weekend, but tentatively I think this feature will be okay to add.

Thanks for putting in the effort to write it up and explain the use case!

[gfredericks]

There's a general issue raised by this change, of call ordering.

You may have noticed that the playbacker function is designed to support different ordering constraints, though it currently only supports one.

I think there's an assumption in this code that new episodes can only be for unrecognized args (rather than, e.g., a scenario where a cassette contains 3 identical calls and you run your test with 4 calls and so it records 1 new one). This seems like a reasonable assumption to me, although I would prefer it made explicit in the documentation.

However I think it gets messy if the other two kinds of ordering constraints (:var and :global) get added, because then you have an ambiguous situation where if you get an unexpected call during playback you have to decide if it's an error because it doesn't match the next call in the cassette or if it's actually a new call that needs to be recorded (and inserted in the middle of the cassette rather than the end?).

Do you know how the ruby library handles this? If I had to guess I would think they probably don't support these other kinds of ordering constraints at all.

[gfredericks]

The TODO should stay here, as there are still use cases that this feature doesn't address.

[gfredericks]

This problem would be better solved in the vcr-clj.clj-http namespace, if possible. The print-method for ::serializable-http-request in that namespace could be enhanced so that http requests are recognized when reading, maybe with some help from the data-readers in the serialization namespace.

The reason for this is that everything in vcr-clj outside of the clj-http namespace is meant to be generic, and so attaching this metadata at this point in the code is inappropriate since it would be applied to all function calls, not just http requests. (there is currently a test failure because of this problem)

[gfredericks]

this calls indexed-cassette on each call, which is unnecessary work since it's being called with the same argument each time

[gfredericks]

I updated the master branch with some test changes and added circle-ci integration.

You'll have to merge/rebase master to get the circleci build to pass, since it expects the lein ci task to be present.

[thegreatape]

@gfredericks Hey, just want to drop a quick note to say thank you for the feedback and that I haven't forgotten this PR. Life and other work obligations have been hoovering up my time, but I should hopefully be able to dig in here later this week or early next. Thanks for your patience!

[gfredericks]

No problem at all