Easier integration with unittest.TestCase

[dwt]

Describe the bug

When using responses with projects, I really like to follow a 'record then replay' policy, that allows me to replay real server responses to my unit tests and rebuild those recordings whenever I need (or have unit tests run against the recordings, and then re-run them without the recordings as slow acceptance tests).

Anyway, this is not hard to do with responses but also not trivial, as the test case integration is not super easy to switch around.

Additional context

So I've built some code like this to ease this integration:

def fixture_path(a_function):
    function_definition_file = Path(a_function.__globals__['__file__'])
    fixtures_folder = (function_definition_file / '../fixtures').resolve()
    fixtures_folder.mkdir(exist_ok=True)

    file_name = a_function.__qualname__ + '.toml'
    path = fixtures_folder / file_name

    return path

def responses(*, record=None, replay=None):

    def wrapper(a_function):
        nonlocal record, replay

        path = fixture_path(a_function)
        if not record and not replay:
            record = not path.is_file()
            replay = not record

        if record:
            return _recorder.record(file_path=path)(a_function)
        elif replay:
            @responses_backend.activate
            @wraps(a_function)
            def activated_function(*args, **kwargs):
                responses_backend._add_from_file(file_path=path)
                return a_function(*args, **kwargs)
            return activated_function

    return wrapper

This achieves several goals:

• Test methods can just be annotated with @responses(). This records the server interaction on the first run, then adds that recording to a file fixtures/$qualified_test_name.toml in the same directory as the file that the test was declared in and replays it on every subsequent run.
• Using the annotation @responses(record=True) override this default and forces a recording as needed.
• @responses(replay=True) forces a replay. (Not so sure about the usefulness while writing this)
• Since the arguments to @responses() are marked as keyword only, forgetting the () to the annotation results in an error and is thus easy to fix. (Alternatively this could also be handled as the default case… now that I think about it.

Anyway, this is what I have come up on short notice, and I think it would be great if responses could provide opinionated code about how to make it easy to integrate it in a test suite.

I would be glad to provide a pull request - but only if you guys are actually interested in merging something like ths.

So what do you think?

Version of responses

0.22.0

Steps to Reproduce

# your code goes here

Expected Result

foo

Actual Result

bar

[beliaev-maksim]

Hello, thanks for the proposal

However, I am -1 on this. First of all, we do not provide a wrapper for reading from files. In one of the discussions me and Mark have already discussed it and we agreed on _add_from_file functionality. This is explicit and could be mixed with standard registry of responses.

You do a recording only once with @_recorder.record and then you completely remove it, so, you explicitly ensure that no real request can go through. In your scenario you still have to modify code, like switching True to False and vice versa but then hide some parts and make it implicit.

Anyway, this idea was discussed by us in the past and I just share a very short summary, hope for understanding

[dwt]

Could you give me some more context on why this is a bad idea? I have had good success in the past to work with an approach like this and even if you don't recommend it as the default approach this could be a valuable alternative approach to support?

[beliaev-maksim]

First of all, we do not provide a wrapper for reading from files. In one of the discussions me and Mark have already discussed it and we agreed on _add_from_file functionality. This is explicit and could be mixed with standard registry of responses.

You do a recording only once with @_recorder.record and then you completely remove it, so, you explicitly ensure that no real request can go through. In your scenario you still have to modify code, like switching True to False and vice versa but then hide some parts and make it implicit.

[dwt]

I am not sure I quite understand you.

a) If you say, you do not provide a wrapper for reading from files, do you mean that @_recorder.record() is deprecated and you want to remove that? Or are you saying, that you don't want to have logic that automatically determines the filename from the test?

b) "You do a recording only once with @_recorder.record and then you completely remove it", that is indeed what I am hoping for, do you say that I misunderstand the way the library works and requests actually still get through? If so, I would like to modify this so no requests can get through. It is my goal to allow tests to run in an a sealed CI environment where none of the real servers are accessible. Perhaps later I would like to run them on an integration server too as integration tests that talk to the real thing, but that is not too important to me right now.

I would appreciate some context on why you think / have agreed on why these are bad ideas?

[dwt]

This is the current version of our integration, just so we have the same context:

def fixture_path(a_function):
    function_definition_file = Path(a_function.__globals__["__file__"])
    fixtures_folder = (function_definition_file / "../fixtures").resolve()
    fixtures_folder.mkdir(exist_ok=True)

    file_name = a_function.__qualname__ + ".toml"
    path = fixtures_folder / file_name

    return path

def responses(a_function=None, *, record=None, replay=None, log=False):
    """Record and replay http interactions that happen through the requests library.

    Use this decorator as the entry point to the responses framework. There are several
    ways to use this decorator:

    from utils.testing import responses
    class MyTest(unittest.TestCase):
        @responses  # equivalent to @responses()
        def test_fnord(self):
            assert something

    The most simle way to use the decorator. Will record a test case on the first run, and
    save it in a `fixtures` folder in the same directory as the test. The recording file is
    named after the fully qualified name of the test function.

    Adding the decorator as `@responses(record=True)` or `@responses(replay=True)` will
    force the respective feature.

    `@responses(replay=True)` is especially helpfull if you just want to use the responses
    API to set your own mocks in code.

    `@responses(log=True)` will enable detailed request logging (independent from `record` 
    or `replay`).

    See https://github.com/getsentry/responses for details on how to use responses directly.

    Args:
        a_function (typing.Callable, optional): Either the decorated function
            (adding the decorator as `@responses` or none, if added as `@responses()`
            to set the `record` or `replay` parameters.). Defaults to None.
        record (bool, optional): Force recording a new trace if set truish. Defaults to None.
        replay (bool, optional): Force replaying existing trace if set truish. Defaults to None.
    """

    import contextlib
    @contextlib.contextmanager
    def detailed_request_level_logging_if_neccessary():
        if not log:
            yield
            return

        from http.client import HTTPConnection
        old_http_connection_debug_level = HTTPConnection.debuglevel
        HTTPConnection.debuglevel = 1

        import logging
        # need to initialize logging, otherwise you will not see anything from requests
        # logging.basicConfig()
        old_generic_log_level = logging.getLogger().level
        logging.getLogger().setLevel(logging.DEBUG)
        requests_log = logging.getLogger("urllib3")
        old_urllib_log_level = requests_log.level
        requests_log.setLevel(logging.DEBUG)
        old_urllib_log_propagate = requests_log.propagate
        requests_log.propagate = True
        try:
            yield
        finally:
            HTTPConnection.debuglevel = old_http_connection_debug_level
            logging.getLogger().setLevel(old_generic_log_level)
            requests_log.setLevel(old_urllib_log_level)
            requests_log.propagate = old_urllib_log_propagate

    def wrapper(a_function):
        nonlocal record, replay

        path = fixture_path(a_function)
        if not record and not replay:
            record = not path.is_file()
            replay = not record

        if record:
            with detailed_request_level_logging_if_neccessary():
                return _recorder.record(file_path=path)(a_function)

        elif replay:

            @responses_backend.activate
            @wraps(a_function)
            def activated_function(*args, **kwargs):
                if path.is_file():
                    responses_backend._add_from_file(file_path=path)

                with detailed_request_level_logging_if_neccessary():
                    return a_function(*args, **kwargs)

            return activated_function

    if a_function:
        assert isinstance(
            a_function, typing.Callable
        ), f"@responses() can only be called with a function as it's first positional argument: {a_function!r}"
        return wrapper(a_function)

    return wrapper

[dwt]

@beliaev-maksim - are you willing to provide further context to my questions? No pressure if you aren't then I can go away, but it is something that I'd like to understand and improve the integration from our side with.

[beliaev-maksim]

recorder.record is used only for recording

all the logic for populating the registry is incorporated into responses.add, responses.get, ..., responses._add_from_file

probably the confusion for you comes from the word replay. In our case we do not execute the cycle, we just populate the registry with responses. Then execution is done via the standard process

[dwt]

@beliaev-maksim In that case it seems I did get you right. That is indeed how this works. So for example a test with this wrapper works like this:

from utils.testing import responses
# This will auto record / replay server communication. Record if no record file is present, then replay from that point forward
@responses  # this is the wrapper depicted above
def test_fnord1():
    response = requests.get(some_url, params=some_params)
    assert response.some_attribute

# This will always record server communication, regardless wether a recording file is already present
@responses(record=True)  # this is the wrapper depicted above
def test_fnord2():
    response = requests.get(some_url, params=some_params)
    assert response.some_attribute

# This will always replay recorded server communication. If no recorded file is present, no file will be added to responses with `responses._add_from_file()`. This is useful to add responses manually inside the test
@responses(replay=True)  # this is the wrapper depicted above
def test_fnord2():
    response = requests.get(some_url, params=some_params)
    assert response.some_attribute

To integrate this - I do get that you do not want to hardcode this as the only sensible way you want to support to work with tests, perhaps because most of the tests you write are not record/replay, but with manually constructed request assertions.

If I where to integrate this into responses I would expect this to work something like this:

from responses.integration import record_and_replay_workflow as responses

@responses
def test_fnord(): ...

Then other workflows can be integrated the same way without preferential treatment.

I guess my main point is, that I would like more complete workflows completely formulated in the library without having to rebuild them from scratch for every project I integrate responses into. That is the point I would like to discuss, the concrete example I gave is just my use case which I would love to upstream if you guys are willing to take it, but the deeper point is indeed that I would like more guidance from responses what correct ways to integrate it would be.

[beliaev-maksim]

again, we discussed the idea of using method wrapper for replaying and we rejected it.

recording obviously could be done only via wrapper. But registry population should be done via a method without wrapper.

[dwt]

I may be getting on your never here - and that is not intended.

But I'd really like to know WHY you rejected the wrapper approach? I have no way to argue for it, if I can't find out what the reasons were that you rejected it the first place? I would take a reference to the bug / pull request in question to read up, but I can't seem to be able to find it myself.