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

testing with betamax? #212

Open
anarcat opened this issue Dec 10, 2019 · 5 comments
Open

testing with betamax? #212

anarcat opened this issue Dec 10, 2019 · 5 comments

Comments

@anarcat
Copy link

anarcat commented Dec 10, 2019

is there a way to test this module alongside betamax?

the idea would be that the cachecontrol module would hit its own cache if present, otherwise bounce back into betamax. I am already using betamax extensively in my project to "mock" HTTP requests (record and replay, really) and it's been quite useful. But when I pair it with cachecontrol, I don't get good results.

either:

  1. i add cachecontrol as an "old_adapter" in the betamax session, which works in the sense that requests never hit the network, but then they never hit the cachecontrol adapter either, because the query gets cached in betamax. even if they would hit cachecontrol, they would need to hit the network first
  2. I mount cachecontrol on top of a betamax session, in which case cachecontrol completely obliterates the betamax adapter and sends requests on the network (instead of tapping in betamax)

Those two possible scenarios are pretty much covered in this ifcase:

cache_adapter = cachecontrol.CacheControlAdapter(cache=FeedContentCacheStorage(self.db_path))
# assume we mount over http and https all at once so check
# only the latter
adapter = self._session.adapters.get('https://', None)
if hasattr(adapter, 'old_adapters'):
    # looks like a betamax session was setup, hook ourselves behind it
    logging.debug('appending cache adapter (%r) to existing betamax adapter (%r)', cache_adapter, adapter)
    adapter.old_adapters['http://'] = cache_adapter
    adapter.old_adapters['https://'] = cache_adapter
else:
    logging.debug('mounting cache adapter (%r)', cache_adapter)
    # override existing adapters to use the cache adapter instead
    self._session.mount('http://', cache_adapter)
    self._session.mount('https://', cache_adapter)

That's pretty much case 1 above, which falls back to case 2 if we don't detect a "betamax" adapter in place.

And of course, it doesn't work.

What I would need is something like betamax's "old_adapters" system. Instead of calling the parent class, CacheControlAdapter.send() should allow the caller to define a list of adapters to call instead. The way betamax does this is fairly simple: there's an old_adapters parameter that you can pass and that it uses for sending its requests..

I have looked into the cachecontrol test suite to figure out if there could be inspiration there, but it seems to "mock" an http adapter instead of delegating requests... Since I don't want to reimplement the entire thing myself (and I can't figure out how to hook betamax in there either), could we add a similar functionality here?

Thanks!

PS: it seems I can mock a parent class but that seems horribly dirty - I am not sure I want to go that far...

@kratsg
Copy link

kratsg commented Mar 16, 2020

Have you been able to figure this out? I ran into this situation.

Here's my hack for now (in testing setup):

    with betamax.Betamax(session).use_cassette('test_cache.test_cache_component'):
        session.mount(session.prefix_url, session.adapters.get('https://', None).old_adapters[session.prefix_url])
        response = session.get(...)

and it seems to work correctly, except it still calls HTTPAdapter instead of relying on requests.adapters.

@anarcat
Copy link
Author

anarcat commented Mar 16, 2020

@kratsg
Copy link

kratsg commented Mar 16, 2020

Note that i found out betamax never calls build_response so you never end up hitting cachecontrol from that. Which is somewhat frustrating.

@anarcat
Copy link
Author

anarcat commented Mar 16, 2020

yep, that's what i found out as well.

@kratsg
Copy link

kratsg commented Mar 16, 2020

this is roughly how close I got

    def mock_factory(betamax_send):
        def wrap_betamax_send(self, request, **kwargs):
            resp = betamax_send(request, **kwargs)
            return session.adapters.get(client.prefix_url, None).build_response(
                request, resp
            )

        return wrap_betamax_send

    with betamax.Betamax(session).use_cassette('test_cache.test_cache_component'):
        mocker.patch(
            'betamax.adapter.BetamaxAdapter.send',
            new=mock_factory(sesssion.adapters.get('https://', None).send),
        )
        client.mount(
            session.prefix_url,
            session.adapters.get('https://', None).old_adapters[session.prefix_url],
        )

problem is that betamax seems to be doing a bunch of extra stuff that I'm unable to inject cachecontrol in correctly.

dbiesecke pushed a commit to dbiesecke/feed2exec that referenced this issue Jul 20, 2021
This was failing because hooking up the cache into the session
completely obliterates our poor old betamax cache. Instead of doing
that, we politely queue the cache layer behind it...

... except that doesn't really work at all, does it. When the betamax
cassette is hit, it does not need the cache, it *is* a cache. So that
change in the controller setup breaks the test_fetch_cache
which *does* expect the cache to "work" (that is, that the body is
"None"), which it isn't when betamax wins.

So we just mark that test as an expected failure for now. The proper
way of doing this would be for the cache controller to be hit first,
and if it doesn't find anything, ask betamax. But I couldn't figure
out how to do this. I asked upstream:

psf/cachecontrol#212

It would require some changes in the cache controller code.

Meanwhile, i tried various horrors to work around this problem. I
tried monkey-patching the base class of CacheControlAdapter so that it
would call betamax, that failed in mysterious ways:

https://stackoverflow.com/a/39311813

Specifically, it told me it couldn't call `send()` on a mock
object. Go figure.

I also tried to monkeypatch my way around this by deriving the
CacheControlAdapter class to inject the betamax class as the called
class, but this ended up in some sort of infinite loop and I got lost.

I *could* have let the test_fetch_cache() function skip betamax and
just talk to the cache. But then it would hit the network, and that
would break autobuilders and all sorts of other assumptions.

So fuck it. The caching layer works well enough, and this fixes the
test suite for now.
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

No branches or pull requests

2 participants