briancurtin / deprecation

A library to handle automated deprecations
Apache License 2.0
88 stars 31 forks source link

fail_if_not_removed decorator does not work for async methods #61

Open chbndrhnns opened 2 years ago

chbndrhnns commented 2 years ago

I am using anyio with asyncio. It seems to me that the decorator messes with the async frameworks and causes the test to be skipped.

Example 1: this test is ignored with "SKIPPED (async def function and no async plugin installed (see warnings))"

import deprecation
import pytest

@deprecation.fail_if_not_removed
@pytest.mark.anyio
async def test_async():
    assert True

Example 2: If I switch the decorators around, I see another message in addition: "coroutine 'test_async' was never awaited":

import deprecation
import pytest

@pytest.mark.anyio
@deprecation.fail_if_not_removed
async def test_async():
    assert True
hemidactylus commented 2 months ago

@chbndrhnns is the issue above still relevant? FYI, I solved the problem by creating my own async version of that decorator. At the moment I keep it in my conftest along with the fixtures (there does not seem to be activity in this repo anymore).

The following may help (except, mypy would have to be silenced with a type: ignore much like for the sync counterpart):

Edit: added minimal support for typing (=> also a customized sync decorator).

import functools
import warnings
from typing import Any, Awaitable, Callable

from deprecation import UnsupportedWarning

def async_fail_if_not_removed(
    method: Callable[..., Awaitable[Any]]
) -> Callable[..., Awaitable[Any]]:
    """
    Decorate a test async method to track removal of deprecated code.

    This is a customized+typed version of the deprecation package's
    `fail_if_not_removed` decorator (see), hastily put together to
    handle async test functions.

    See https://github.com/briancurtin/deprecation/issues/61 for reference.
    """

    @functools.wraps(method)
    async def test_inner(*args: Any, **kwargs: Any) -> Any:
        with warnings.catch_warnings(record=True) as caught_warnings:
            warnings.simplefilter("always")
            rv = await method(*args, **kwargs)

        for warning in caught_warnings:
            if warning.category == UnsupportedWarning:
                raise AssertionError(
                    (
                        "%s uses a function that should be removed: %s"
                        % (method, str(warning.message))
                    )
                )
        return rv

    return test_inner

def sync_fail_if_not_removed(method: Callable[..., Any]) -> Callable[..., Any]:
    """
    Decorate a test sync method to track removal of deprecated code.

    This is a typed version of the deprecation package's
    `fail_if_not_removed` decorator (see), with added minimal typing.
    """

    @functools.wraps(method)
    def test_inner(*args: Any, **kwargs: Any) -> Any:
        with warnings.catch_warnings(record=True) as caught_warnings:
            warnings.simplefilter("always")
            rv = method(*args, **kwargs)

        for warning in caught_warnings:
            if warning.category == UnsupportedWarning:
                raise AssertionError(
                    (
                        "%s uses a function that should be removed: %s"
                        % (method, str(warning.message))
                    )
                )
        return rv

    return test_inner
chbndrhnns commented 2 months ago

Yes, the issue persists. Thanks so much for sharing your workaround. I have not used the library due to this problem.

briancurtin commented 2 months ago

@hemidactylus if you wanted to get that into a PR I'd be happy to look at it and merge/release to get this out there. Sorry for delay on getting to other issues and PRs; I can probably make some time to get a bunch of them taken care of in a batch.