jamescooke/flake8-aaa

`pytest.warns` should be treated as `pytest.raises`

lyz-code opened this issue ยท 7 comments

Describe the bug
I have a test that want's to assert that a function warns a user with a UserWarning. but flake8-aaa does not see it as an act block.

    def test_fix_files_issues_warning(self, tmp_path: Path) -> None:
        """
        Given: A file to fix
        When: Using the old signature
        Then: A warning is issued
        """
        test_file = tmp_path / "source.yaml"
        test_file.write_text("program: yamlfix")

        with pytest.warns(UserWarning, match="yamlfix/pull/182"):
            fix_files([str(test_file)])

Before I used the # act comment, but not it looks like black doesn't allow to have white lines between the context manager and the first line, so the next snippet get's automatically converted to the following

        with pytest.warns(UserWarning, match="yamlfix/pull/182"):

            fix_files([str(test_file)]) # act
        with pytest.warns(UserWarning, match="yamlfix/pull/182"):
            fix_files([str(test_file)]) # act

Thus joining the arrange and act blocks.

Share debugging output

flake8 --version:

4.0.1 (aaa: 0.12.2, dlint: 0.13.0, flake-mutable: 1.2.0, flake8-annotations: 2.9.1, flake8-annotations-
complexity: 0.0.7, flake8-bugbear: 22.12.6, flake8-comprehensions: 3.10.1, flake8-debugger: 4.1.2,
flake8-docstrings: 1.6.0, pydocstyle: 6.1.1, flake8-eradicate: 1.4.0, flake8-expression-complexity:
0.0.11, flake8-pytest: 1.4, flake8-pytest-style: 1.6.0, flake8-variables-names: 0.0.5, flake8_simplify:
0.19.3, flake8_typing_imports: 1.12.0, mccabe: 0.6.1, naming: 0.13.2, pycodestyle: 2.8.0, pyflakes:
2.4.0, pylint: 3.0.0-a4, use-fstring-format: 1.4, use-fstring-percent: 1.4, use-fstring-prefix: 1.4)

Please share the Python version and platform you are using:

  • Python version (output of python --version): Python 3.9.10
  • Platform (GNU Linux / Mac OSX / Windows): Linux

Expected behavior

pytest.warns is treated as a recognized Act block similar to pytest.raises.

Thanks for this - I didn't know about pytest.warns() ๐Ÿ‘๐Ÿป (docs are here: https://docs.pytest.org/en/7.2.x/how-to/capture-warnings.html#recwarn)

I think this makes total sense to have recognised as an Act block.

If you point me in the direction on how to fix this, I may be able to do a PR

Sorry for being slow @lyz-code - thanks for offering help ๐Ÿ™๐Ÿป

Two things that would help...

  • Are you able to install from a git branch? If so, could you try the extend-pytest-contexts branch in #199? I've added an undocumented feature to handle pytest.warns() context managers. If you let me know that works, that would be great - this will be released in 0.13.0 hopefully ๐Ÿคž๐Ÿป
  • I need to build out the "good" examples for context managers here: https://github.com/jamescooke/flake8-aaa/blob/master/examples/good/test_with_statement.py to include at least one real and valid test each for pytest.warns() and pytest.deprecated_call(). If you have any examples that can be added please suggest them here, or PR them directly onto the extend-pytest-contexts feature branch ๐Ÿ™๐Ÿป .

Finally - I'm hoping that Black won't completely muddy the water for us on this fix. Please see if you can use black<23 while checking on anything above.

I'm intending to fix this issue insolation first, then work on the Black newlines / context manager issues :hurtrealbad:

Sorry for being slow ...

It's fine! it's enough that you coded the package and maintain it, there's never enough time to code

I'll try to save some time between today and tomorrow to deal with this, let's hope life doesn't get in the way

Are you able to install from a git branch? If so, could you try the extend-pytest-contexts branch

I can confirm that with the extend-pytest-contexts the next code:

    def test_fix_files_issues_warning(self, tmp_path: Path) -> None:
        """
        Given: A file to fix
        When: Using the old signature
        Then: A warning is issued
        """
        test_file = tmp_path / "source.yaml"
        test_file.write_text("program: yamlfix")
        with pytest.warns(UserWarning, match="yamlfix/pull/182"):

            fix_files([str(test_file)])  # act

Raises the next exception:

tests/unit/test_services.py
    69:   9 AAA03 expected 1 blank line before Act block, found none [flake8-aaa]
  with pytest.warns(UserWarning, match="yamlfix/pull/182"):
  ^
    70:   1 AAA05 blank line in block [flake8-aaa]
  
  ^

The next one raises no error:

    def test_fix_files_issues_warning(self, tmp_path: Path) -> None:
        """
        Given: A file to fix
        When: Using the old signature
        Then: A warning is issued
        """
        test_file = tmp_path / "source.yaml"
        test_file.write_text("program: yamlfix")

        with pytest.warns(UserWarning, match="yamlfix/pull/182"):
            fix_files([str(test_file)])  # act

And if I remove the # act comment, no error is raised either \o/ thanks!

I need to build out the "good" examples for context managers

There you go :) :

import pytest
import warnings

def test_deprecation_warning():
    with pytest.deprecated_call():
        warnings.warn("deprecate warning", DeprecationWarning)
        

def test_user_warning():
    with pytest.warns(UserWarning):
        warnings.warn("my warning", UserWarning)

Thanks @lyz-code for confirming the feature branch works! ๐ŸŽ‰

And thanks for those good examples - perfect and compact. I'll use them in the feature branch.

Hopefully I'll be able to get the branch merged tomorrow - and then get 0.13 out ๐Ÿคž๐Ÿป ... As you said - let's hope life doesn't get in the way - and that the packaging / release process that hasn't been used for over a year isn't borked either! ๐Ÿ˜ฌ

Managed to hack through some packaging issues and got 0.13.0 out ๐Ÿ˜… ๐Ÿ’ช๐Ÿป

https://pypi.org/project/flake8-aaa/0.13.0/

I'm going to close this now, but please reopen / or open a new one if there are any problems. Thanks for all your help with this ๐Ÿ™๐Ÿป