getsentry / responses

A utility for mocking out the Python Requests library.
Apache License 2.0
4.1k stars 351 forks source link

Inconsistent behaviour of assert_all_requests_are_fired between decorator and context manager #726

Open andrew-cybsafe opened 1 month ago

andrew-cybsafe commented 1 month ago

Describe the bug

When a RequestsMock() is used as a context manager, assert_all_requests_are_fired is defaulted to True and an exception is raised at the end of the context block if a response is unused. However, using the @responses.activate decorator on a test function defaults assert_all_requests_are_fired to False leading to inconsistent behaviour.

Additional context

As a workaround, I have tried setting responses.mock.assert_all_requests_are_fired = True in conftest.py or just after an import of requests. However, this does not change the behaviour. The issue appears to be that the value in the default mock is never used as it gets defaulted by a default parameter in activate(), see: code.

Version of responses

0.25.3

Steps to Reproduce

import responses
import requests

def test_my_api():
    with responses.RequestsMock() as rsps:
        rsps.add(
            responses.GET,
            "http://twitter.com/api/1/foobar",
            body="{}",
            status=200,
            content_type="application/json",
        )

@responses.activate
def test_my_api_2():
    responses.add(
        responses.GET,
        "http://twitter.com/api/1/foobar",
        body="{}",
        status=200,
        content_type="application/json",
    )

Expected Result

Both test cases to fail with:

AssertionError: Not all requests have been executed [('GET', 'http://twitter.com/api/1/foobar')]

Actual Result

First test case fails. Second test case passes.

markstory commented 1 month ago

Changing the default values is not something we can easily do at this point in time as it will break existing assumptions in userland code.

I have tried setting responses.mock.assert_all_requests_are_fired = True in conftest.py or just after an import of requests.

Because of fixes we had to make to fix nested scopes, I don't think this is currently working. But it should be possible to have it work.

andrew-cybsafe commented 1 month ago

@markstory I've opened PR https://github.com/getsentry/responses/pull/731 for this.