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

fail_if_not_removed decorator does not work for async methods #61

Open
chbndrhnns opened this issue Apr 26, 2022 · 3 comments
Open

fail_if_not_removed decorator does not work for async methods #61

chbndrhnns opened this issue Apr 26, 2022 · 3 comments

Comments

@chbndrhnns
Copy link

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
Copy link

hemidactylus commented Jun 13, 2024

@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
Copy link
Author

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

@briancurtin
Copy link
Owner

@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.

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

3 participants