Pytest: Deprecation of terminalreporter.writer causes plugin tests to fail.

[BeyondEvil]

After the deprecation of terminalreporter.writer pytest plugin tests using pytester are getting the deprecation warning, even tho the plugin is not using terminalreporter at all.

Using pytest version 5.3.5 it works as expected.

Tested on both Linux and Mac using the below example test.

pytest_plugins = ("pytester",)

def test_base(testdir, recwarn):
    testdir.makepyfile("def test_pass(): pass")
    testdir.runpytest()
    print(recwarn.list[0])
    assert len(recwarn) == 0
    assert True

test outcome

# pytest -s -ra -v
=========================================================================================== test session starts ============================================================================================
platform linux -- Python 3.7.6, pytest-5.4.1, py-1.8.1, pluggy-0.13.1 -- /usr/local/bin/python
cachedir: .pytest_cache
rootdir: /pytest-dev/pytest-playground/tests, inifile: pytest.ini
collected 1 item                                                                                                                                                                                           

test_selenium.py::test_base =========================================================================================== test session starts ============================================================================================
platform linux -- Python 3.7.6, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
rootdir: /tmp/pytest-of-root/pytest-5/test_base0
collected 1 item

test_base.py .                                                                                                                                                                                       [100%]

============================================================================================ 1 passed in 0.01s =============================================================================================
{message : PytestDeprecationWarning('TerminalReporter.writer attribute is deprecated, use TerminalReporter._tw instead at your own risk.\nSee https://docs.pytest.org/en/latest/deprecations.html#terminalreporter-writer for more information.'), category : 'PytestDeprecationWarning', filename : '/usr/local/lib/python3.7/site-packages/_pytest/terminal.py', lineno : 289, line : None}
FAILED

================================================================================================= FAILURES =================================================================================================
________________________________________________________________________________________________ test_base _________________________________________________________________________________________________

testdir = <Testdir local('/tmp/pytest-of-root/pytest-5/test_base0')>, recwarn = WarningsRecorder(record=True)

    def test_base(testdir, recwarn):
        testdir.makepyfile("def test_pass(): pass")
        testdir.runpytest()
        print(recwarn.list[0])
>       assert len(recwarn) == 0
E       assert 1 == 0
E         +1
E         -0

/pytest-dev/pytest-playground/tests/test_selenium.py:10: AssertionError
========================================================================================= short test summary info ==========================================================================================
FAILED test_selenium.py::test_base - assert 1 == 0
============================================================================================ 1 failed in 0.18s =============================================================================================

I'm seeing tests fail in pytest-html which only uses terminalreporter.write_sep() in a hook, and in pytest-variables which does not use terminalreporter at all.

It's failing for both python 3.6 and 3.7. Example

[nicoddemus]

Thanks a lot @BeyondEvil!

Hmmm this is tricky, the deprecation is happening because runpytest() is registering the terminal plugin, and when it does that pytest is looking at all members of TerminalReporter to gather hooks and fixtures, which then triggers the warning when safe_getattr gets called for writer:

(Sorry for the screenshot, I'm short on time).

Not sure how to approach this, suggestions are welcome!

[BeyondEvil]

The pytest internals are very much a black box to me. But I do at least want to _try_ and help with a solution (more of a hack maybe).

How about adding below snippet in pytester#inline_run.

        import warnings
        from _pytest.warnings import _setoption

        arg = "ignore:TerminalReporter.writer:pytest.PytestDeprecationWarning"
        _setoption(warnings, arg)

I'm happy to do a PR if there's no better solution.

[bluetech]

This will be fixed by #6898, which includes a change to skip the getattr on properties. Not sure what @RonnyPfannschmidt's plan is for that PR.

[RonnyPfannschmidt]

Good reminder, I want that sorted soon

[ssbarnea]

@RonnyPfannschmidt I am looking forwards to see this fixed as it breaking other projects like pytest-html.

[RonnyPfannschmidt]

The Bugfix was released

[BeyondEvil]

I'm probably doing something wrong. But the error described in this issue still happens with pytest 5.4.2. @RonnyPfannschmidt

[RonnyPfannschmidt]

Then I misunderstood something and a additional fix is needed

[BeyondEvil]

Let me know if there's something I can help with @RonnyPfannschmidt

[RonnyPfannschmidt]

Atm anything, currently I don't have much time for pytest due to global reasons

[BeyondEvil]

@bluetech #6898 is merged. It didn't solve this issue however. Any clues what else could be done?

[bluetech]

@BeyondEvil it isn't merged yet, unless I misunderstand.

For reference, the issue is this:

TerminalReporter has an property writer that has been deprecated in pytest 5.4.0. So every time it is accessed, a deprecation warning is triggered.

TerminalReporer is a plugin. A plugin can define fixtures. During the collection phase, pytest collects fixtures by sifting through all attributes of an object (plugin in this case), accessing them with getattr and checking if they are fixtures. If the attribute is a property, its actually runs, and in writers case triggers the warning.

What @RonnyPfannschmidt's change does is: before probing a given attribute, check if it is a property, and if so, skip it. The change itself is small and can be extracted to its own PR. But before we do so, we should check:

• Is there some reasonable possibility/use-case for using a property to define a fixture? (probably not)
• Does the fix adversely impact performance? (probably not)

[nicoddemus]

Is there some reasonable possibility/use-case for using a property define a fixture? (probably not)

I agree, but with tongue-in-cheek, I'm sure there's a test suite out there which has a property, that when accessed, returns a function marked as a fixture. 😆

[RonnyPfannschmidt]

Time for a breaking change

[nicoddemus]

Do you plan to get to this @RonnyPfannschmidt? Otherwise we will just remove the attribute in 6.1.

[RonnyPfannschmidt]

I'm going to try and get to it this evening

[joshua-badger]

Hello all, just chiming in to say we are still seeing this in pytest 5.4.3. Has there been any movement on this? Is there help we can provide?

[RonnyPfannschmidt]

I do not get to it @nicoddemus can you pick up the removal?

[nicoddemus]

Sure, no problem.

[nicoddemus]

I was about to remove this for 6.0, but then thought it was better to just wait to remove this in 6.1: I don't see much reason to bypass our usual deprecation/removal process.

[the-wondersmith]

I don't know if anyone else is still experiencing this issue, but as far as I can tell it's pretty much exactly what @bluetech said - the issue stems from something in one of the JetBrains plugins trying to access the writer attribute of TerminalReporter.

I have no idea what the "correct" fix is, and I'm personally more willing to patch the plugin than I am to patch the local instance of PyTest, so I came up with this:

from typing import Any
from _pytest import compat


def patched_safe_getattr(object: Any, name: str, default: Any) -> Any:
    """ PyCharm has a PyTest plugin that *will not* stop
        throwing deprecation warnings, pretty much no matter
        what you do

        So, instead of accepting that as inevitable, we're gonna
        monkeypatch the method that trips the deprecation warning
    """
    try:
        if all(("TerminalReporter".casefold() in str(object).casefold(), name == "writer")):
            return getattr(object, "_tw", default)
        return getattr(object, name, default)
    except compat.TEST_OUTCOME:
        return default


compat.safe_getattr = patched_safe_getattr

And added it immediately after the pytest imports in:

{PyCharm.app}/plugins/python/helpers/coverage_runner/run_coverage.py
and
{PyCharm.app}/plugins/python/helpers/pycharm/_jb_pytest_runner.py

The deprecation warning doesn't get thrown because the plugin (and everything else that gets loaded, really) is now handed TerminalWriter._tw (per the deprecation warning) any time it tries to access TerminalWriter.writer whether it likes it or not.

Like I said, it may not be an ideal solution, but it is a functional solution - at least until someone smarter than me comes up with a more proper fix. 😅

[The-Compiler]

@the-wondersmith That seems unrelated to plugin tests using pytester/testdir (which this issue is about). That one should probably be reported to JetBrains if that hasn't happened yet.

[BeyondEvil]

I can confirm this is 100% unrelated to JetBrain.

[the-wondersmith]

I can confirm this is 100% unrelated to JetBrain.

Apologies for the off topic comment then. It's been a recurring issue for me personally and I'd not been able to find a workable solution for it.

[nicoddemus]

Just for clarification, the cause is outlined in https://github.com/pytest-dev/pytest/issues/6936#issuecomment-600369633: basically when listing all members of TerminalReporter, we access the writer attribute to inspect if it is a fixture, which triggers the warning.

We could implement a somewhat convoluted fix to bypass descriptor/properties from that check, but we decided that it is worth introducing this change given that 6.0 and 6.1 are right around the corner, which by then TerminalReporter.writer will be removed and the problem will go away.

[nicoddemus]

Fixed by #7660