Don't save requests from decorated tests if decorated test fails

[dan-passaro]

As the title says: Don't save requests from decorated tests if decorated test fails

Does VCR already do this? If so awesome, and you guys should definitely say so in the README somewhere. Otherwise this seems like a solid feature. We have one test in particular where our 3rd party goes down more often than I'd care to admit, and if that bad response gets saved that will be quite aggravating.

[colonelpanic8]

Are you not tracking your http requests with source control? Should be easy enough to revert to an old version if you are...

I'm not sure that having vcrpy catch exceptions is a great idea.

[dan-passaro]

I'm not version controlling the requests, no. I assume you're talking about the actual vcr cassette file.

I'm not saying vcr should catch exceptions, but that it should not save the cassette file if an exception is raised, e.g.

try:
    test_func()
except Exception:
    delete_or_dont_flush_cassettes_or_whatever()
    raise

This is only assuming that vcr would currently write a cassette file even if a test fails, which I am not sure is true yet as I haven't tested. Luckily our test build workspace currently has a good cassette saved, whether or not that's a happy accident.

[dan-passaro]

I suppose this is too harsh of a generalization if other users have more reliable 3rd parties and want to do TDD against saved cassettes (so they want their cassettes to save even when their test fails). In my case it's that sometimes the request itself fails, although the test and code under test have not changed in a long time, so we get spurious failures in our build. Currently vcr is doing a good job at getting more consistent green dots out of our CI system but if the cassette winds up being bad it would be good at getting consistent red dots.

I know deleting the bad cassette is a solution but I'm wondering if there's an easier way. If having that behavior enabled by default is too much, is there any interest in adding a flag to do basically the same thing but as an opt-in?

[kevin1024]

Hmm, interesting idea. I think this would involve some deeper integration with the testing framework than we currently do. You could probably get something going like this in your testing framework's setup/teardown methods. Flag test failures and blow away the cassette file for that failure or something. VCR doesn't really know that it's executing inside a test or anything, so it can't know if the test fails or passes.

[dan-passaro]

I don't think it's too complex and I would be surprised if I'm the only one with this use case. Showing that the decorator doesn't need to know it's running a test:

# coding: utf-8
from __future__ import (absolute_import, division, print_function,
                        unicode_literals)
import functools
import unittest

def decorator(func):
    @functools.wraps(func)
    def wrapper(*args, **kwargs):
        try:
            return func(*args, **kwargs)
        except Exception:
            print("Something bad happened, don't cassette")
            raise
    return wrapper

class SomeTest(unittest.TestCase):

    @decorator
    def test_foo(self):
        self.assertTrue(False)

Saving this as test_thing.py enables this shell session:

$ python -m unittest test_thing
Something bad happened, don't cassette
F
======================================================================
FAIL: test_foo (test_thing.SomeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_thing.py", line 12, in wrapper
    return func(*args, **kwargs)
  File "test_thing.py", line 23, in test_foo
    self.assertTrue(False)
AssertionError: False is not true

----------------------------------------------------------------------
Ran 1 test in 0.000s

FAILED (failures=1)

You can see the message successfully prints as expected, so I don't think this has to be written in a unittest-specific way.

The logic can be triggered based on a save_on_exception= keyword argument, for example, which would default to True to maintain current behavior.

[agriffis]

@dan-passaro If you'd like to put together a PR for this, I'm sure we'd appreciate it!

[neozenith]

A lot of changes have happened to VCRpy since this ticket was opened. As this ticket has become stale and we have a lot of old tickets that need to be groomed, I'm closing this for now to make it easier to see what is still relevant.

However if it is still needed, please feel free to re-open or create a new ticket.

Thanks! 🙏