responses icon indicating copy to clipboard operation
responses copied to clipboard

Inconsistent behaviour of assert_all_requests_are_fired between decorator and context manager

Open andrew-cybsafe opened this issue 1 year ago • 2 comments

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.

andrew-cybsafe avatar Jul 04 '24 16:07 andrew-cybsafe

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.

markstory avatar Jul 08 '24 20:07 markstory

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

andrew-cybsafe avatar Jul 16 '24 16:07 andrew-cybsafe