kevin1024 / vcrpy

Automatically mock your HTTP interactions to simplify and speed up testing
MIT License
2.69k stars 387 forks source link

[CORE BUG][Verified Can Replicate] Wrong number of requests recorded under new_episodes mode #753

Open vickyliin opened 1 year ago

vickyliin commented 1 year ago

AFAIK, when allow_playback_repeats is False, the number of requests should be equal to the length of cassette data. But when we use new_episodes mode with same/matched requests called several times, the number of cassette data is smaller than the number of requests.

This is the same problem as mentioned here: https://github.com/kevin1024/vcrpy/issues/245 I changed the reproduce script to httpx so it's easier to count the number of requests: https://colab.research.google.com/gist/vickyliin/cf06ea0dc0f7f2f8a1b5c30bb8f3909d/vcrpy-bug.ipynb

In the new_episodes context, the number of reuqests is 7, while the length of cassette.data is only 4

The cause is cassette.append doesn't set play_counts just like #245 mentioned. When a new and matched request calls, it uses the previously appended cassette data. So the total number of cassette.data for the above script is 1 (old) + 6/2 (new episodes) = 4

For now I use the following code as a hotfix:

class FixedCassette(Cassette):
    def append_and_play(self, request, response) -> None:
        prev_len_data = len(self.data)
        super().append(request, response)
        is_appended = len(self.data) == prev_len_data + 1
        if is_appended:
            self.play_counts[prev_len_data] += 1

    def _load(self) -> None:
        super()._load()
        self.append = self.append_and_play
        self._load = None

context = vcr.use_cassette(...)
context.cls = FixedCassette
wich context:
    run_requests()

Related code:

Records are only used when no play_count https://github.com/kevin1024/vcrpy/blob/69de3886494d381c2c1a6620ea9a87dcd4df0d07/vcr/cassette.py#L266-L274

append does not set play_count https://github.com/kevin1024/vcrpy/blob/69de3886494d381c2c1a6620ea9a87dcd4df0d07/vcr/cassette.py#L234-L247

append is also used when loading cassette so we can't set play_count in append directly https://github.com/kevin1024/vcrpy/blob/69de3886494d381c2c1a6620ea9a87dcd4df0d07/vcr/cassette.py#L342-L348

For other modes, we can't see this problem because rewound is False (or it's ALL mode) https://github.com/kevin1024/vcrpy/blob/69de3886494d381c2c1a6620ea9a87dcd4df0d07/vcr/cassette.py#L262-L264

vickyliin commented 1 year ago

I'm trying to make a PR to fix this issue, but found play_count is heavily relied on in the tests to check if a record is newly added or being played.

I'm wondering if

vickyliin commented 1 year ago

This bug is pretty annoying because the cassette keeps growing after each time we run the code, until no new repeated requests is making. It takes log(n) runs to make the cassette stable

vickyliin commented 1 year ago

I'm willing to help. Anyone can make a decision? Anything else can I do now?