betamaxpy / betamax

A VCR imitation designed only for python-requests.
https://betamax.readthedocs.io/en/latest/
Other
562 stars 61 forks source link

Interest in additional hooks? #178

Closed jarhill0 closed 4 years ago

jarhill0 commented 4 years ago

A recently merged change in PRAW (which uses Betamax for its integration tests) hacks in an "after init" hook:

def add_init_hook(original_init):
    """Wrap an __init__ method to also call some hooks."""

    @wraps(original_init)
    def wrapper(self, *args, **kwargs):
        original_init(self, *args, **kwargs)
        dispatch_hooks("after_init", self)

    return wrapper

Cassette.__init__ = add_init_hook(Cassette.__init__)

def init_hook(cassette):
    if cassette.is_recording():
        pytest.set_up_record()  # dynamically defined in __init__.py

Cassette.hooks["after_init"].append(init_hook)

source code in PRAW repo

For PRAW's tests, this hook is useful because it triggers at the time .use_cassette() is called, which in the case of PRAW's tests signifies that network requests will happen soon but they haven't started yet. This hook allows us to modify the test environment slightly before any requests are made, based on whether a certain cassette has been recorded or not.

Rather than having to monkeypatch Betamax, it would be better for PRAW if this hook existed natively in Betamax. Would this be a welcome feature in Betamax? If so, I'll be happy to write the feature.

sigmavirus24 commented 4 years ago

I don't fully understand the necessity for these kinds of hooks.

Right now, all of Betamax's hooks are there to allow modification of the request or response data. Introducing a whole different set of hooks seems like it might confound things a bit.

jarhill0 commented 4 years ago

Right now, all of Betamax's hooks are there to allow modification of the request or response data.

Yes. For our purposes (modifying behavior slightly based on whether a cassette is about to be recorded or about to be played back), having a hook that occurs per-cassette rather than per-request (and before any request) is useful. In case you're interested in reading the relevant commit, it's here, but I suspect that it won't be very interesting or immediately comprehensible.

a whole different set of hooks

I'm only interested in this one particular hook… 😁


In general, I think more hooks makes Betamax more powerful. But perhaps it removes some simplicity.

sigmavirus24 commented 4 years ago

I'm inclined to make this hook more of a notification style than something that can modify things. What behaviour are you considering modifying?

jarhill0 commented 4 years ago

In my case, I'm not modifying Betamax behavior, though I am reading its state to determine whether the cassette is about to record or about to be played back. In the case that it's about to be played back, we modify the class being tested to select the appropriate authentication scheme. Once the cassette is recorded, the auth scheme doesn't matter, because the cassette will play back correctly either way.

sigmavirus24 commented 4 years ago

Got it. I think we could probably add a hook or two that basically signal: