pytest-dev / pytest-bdd

BDD library for the pytest runner
https://pytest-bdd.readthedocs.io/en/latest/
MIT License
1.31k stars 221 forks source link

PytestAssertRewriteWarning breaks test_apply_tag_hook #453

Closed musicinmybrain closed 4 days ago

musicinmybrain commented 3 years ago

This is a bit of a strange one, in that it’s hard to reproduce. I’m the maintainer of the python-pytest-bdd package in Fedora Linux, and I’m seeing a test failure associated with Fedora’s update to pip 21.3.

=================================== FAILURES ===================================
_____________________________ test_apply_tag_hook ______________________________

testdir = <Testdir local('/tmp/pytest-of-mockbuild/pytest-0/test_apply_tag_hook0')>

    def test_apply_tag_hook(testdir):
        testdir.makeconftest(
            """
            import pytest

            @pytest.hookimpl(tryfirst=True)
            def pytest_bdd_apply_tag(tag, function):
                if tag == 'todo':
                    marker = pytest.mark.skipif(True, reason="Not implemented yet")
                    marker(function)
                    return True
                else:
                    # Fall back to pytest-bdd's default behavior
                    return None
        """
        )
        testdir.makefile(
            ".feature",
            test="""
        Feature: Customizing tag handling

            @todo
            Scenario: Tags
                Given I have a bar

            @xfail
            Scenario: Tags 2
                Given I have a bar
        """,
        )
        testdir.makepyfile(
            """
            from pytest_bdd import given, scenarios

            @given('I have a bar')
            def i_have_bar():
                return 'bar'

            scenarios('test.feature')
        """
        )
        result = testdir.runpytest("-rsx")
        result.stdout.fnmatch_lines(["SKIP*: Not implemented yet"])
>       result.stdout.fnmatch_lines(["*= 1 skipped, 1 xpassed * =*"])
E       Failed: nomatch: '*= 1 skipped, 1 xpassed * =*'
E           and: '============================= test session starts =============================='
E           and: 'platform linux -- Python 3.10.0, pytest-6.2.5, py-1.10.0, pluggy-1.0.0'
E           and: 'rootdir: /tmp/pytest-of-mockbuild/pytest-0/test_apply_tag_hook0'
E           and: 'plugins: bdd-4.1.0'
E           and: 'collected 2 items'
E           and: ''
E           and: 'test_apply_tag_hook.py sX                                                [100%]'
E           and: ''
E           and: '=============================== warnings summary ==============================='
E           and: '../../../../usr/lib/python3.10/site-packages/_pytest/config/__init__.py:1114'
E           and: '  /usr/lib/python3.10/site-packages/_pytest/config/__init__.py:1114: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: pytest_bdd'
E           and: '    self._mark_plugins_for_rewrite(hook)'
E           and: ''
E           and: '-- Docs: https://docs.pytest.org/en/stable/warnings.html'
E           and: '=========================== short test summary info ============================'
E           and: 'SKIPPED [1] ../../../../builddir/build/BUILDROOT/python-pytest-bdd-4.1.0-6.fc36.x86_64/usr/lib/python3.10/site-packages/pytest_bdd/scenario.py:163: Not implemented yet'
E           and: '=================== 1 skipped, 1 xpassed, 1 warning in 0.01s ==================='
E       remains unmatched: '*= 1 skipped, 1 xpassed * =*'

result     = <RunResult ret=ExitCode.OK len(stdout.lines)=17 len(stderr.lines)=0 duration=0.04s>
testdir    = <Testdir local('/tmp/pytest-of-mockbuild/pytest-0/test_apply_tag_hook0')>

/builddir/build/BUILD/pytest-bdd-4.1.0/tests/feature/test_tags.py:162: Failed

All of the other tests still pass. As you can see, the glob '*= 1 skipped, 1 xpassed * =*' does not match = 1 skipped, 1 xpassed, 1 warning in 0.01s = due to the unexpected PytestAssertRewriteWarning. I’m not sure why this should be triggered by an update of pip.

Annoyingly, I can’t seem to reproduce this in a plain Python 3.10 virtualenv, using pip 21.3 and pip-installed dependency instead of Fedora-packaged ones.

I found that pytest-cov had a similar issue several years ago and worked around it by adding PYTEST_DONT_REWRITE to the module docstring pytest_cov/__init__.py.

I can confirm that adding PYTEST_DONT_REWRITE to the docstring in pytest_bdd/__init__.py does fix the unexpected warning, but I don’t really understand the pytest rewriting system, so I don’t know if this would have unintended consequences.

Alternatively, either skipping test_apply_tag_hook or loosening the glob very subtly to '*= 1 skipped, 1 xpassed* =*' can work around the issue.

For now, I’m loosening the glob in the test.

I’m curious:

  1. if you have any better understanding of why this might be happening than I do
  2. if you have any thoughts on the downstream options
  3. if you think pytest-bdd needs any changes upstream based on this report

Thanks!

hroncok commented 3 years ago

The new version of pip does in-tree builds by default. Maybe some files exist twice on the sys.path now and that triggers the warning?

musicinmybrain commented 3 years ago

The new version of pip does in-tree builds by default. Maybe some files exist twice on the sys.path now and that triggers the warning?

Thanks, it does appear to be related to that change. As a hacky test (Fedora-specific RPM macro talk ahead) I can replace %pyproject_wheel with its expansion and add --use-deprecated=out-of-tree-build to the pip arguments, such that the whole expansion looks like:

%_set_pytest_addopts
mkdir -p "%{_pyproject_builddir}"
CFLAGS="${CFLAGS:-${RPM_OPT_FLAGS}}" LDFLAGS="${LDFLAGS:-${RPM_LD_FLAGS}}" TMPDIR="%{_pyproject_builddir}" \
%{__python3} -m pip wheel --wheel-dir %{_pyproject_wheeldir} --no-deps --use-pep517 --no-build-isolation --disable-pip-version-check --no-clean --progress-bar off --verbose --use-deprecated=out-of-tree-build .

and the warning disappears. Obviously, that’s not a practical approach—both because hard-coding the expansion is not ideal and because, more importantly, the --use-deprecated=out-of-tree-build option is supposed to go away at some point—but it does narrow down the cause, even though I have still not managed to reproduce the warning outside of a Fedora RPM build.

musicinmybrain commented 3 years ago

(Edited my previous comment to remove some things that were not true, because I accidentally stopped building with pip 21.3 while testing.)

musicinmybrain commented 3 years ago

These are things that do not work around the warning:

mv pytest_bdd _pytest_bdd.noimport
%tox
mkdir -p _empty && cd _empty
%pytest ../tests
rm -rf pytest_bdd
%tox

These are things that do work around the warning:

mkdir -p _empty && cp -rp tests *.ini _empty && cd _empty
%tox

(I’m now doing the above in the Fedora package instead of loosening the glob.)

mkdir -p _empty && cp -rp tests _empty && cd _empty
%pytest
jsa34 commented 4 days ago

Hello @musicinmybrain!

I appreciate it has been a long time, but is this still an issue?

musicinmybrain commented 4 days ago

Hello @musicinmybrain!

I appreciate it has been a long time, but is this still an issue?

Great question! Thanks for checking in on this issue.

I just tried removing the workaround described in https://github.com/pytest-dev/pytest-bdd/issues/453#issuecomment-944385209 and test-building RPM packages of pytest-bdd 7.3.0 in Fedora Rawhide/42 and 41 (with pip 24.2 and pytest 8.3.3), 40 (with pip 23.3.2 and pytest 7.4.3), and 39 (with pip 23.2.1 and pytest 7.3.2). All succeeded.

I suppose that means that this has been fixed somewhere. 🤷

jsa34 commented 4 days ago

Fab! If it appears as an issue feel free to raise a new issue or reopen