Pytest: RemovedInPytest4Warning via _pytest\compat.py:321

[blueyed]

There are many RemovedInPytest4Warnings with pytest's own tests, e.g.:

c:\projects\pytest\.tox\py36\lib\site-packages\_pytest\compat.py:321: RemovedInPytest4Warning: usage of Session.Class is deprecated, please use pytest.Class instead
360  return getattr(object, name, default)
361

(via https://ci.appveyor.com/project/pytestbot/pytest/builds/19474342/job/c83h5n8ne3snvf8o#L359)

Are those expected?

I would assume pytest should not causes warnings itself.

[nicoddemus]

In most cases it is inevitable because we still need to test deprecated features.

[blueyed]

I see.. there are more than 130 of them though.
Maybe pytest's tests should be configured to ignore those warnings then maybe?

[nicoddemus]

Maybe pytest's tests should be configured to ignore those warnings then maybe?

Hmmm yep, makes sense.

[RonnyPfannschmidt]

@nicoddemus this is a bug in plugin factory parsing - we souldnt parse elements that are CompatProperties, or remove those things already

[nicoddemus]

We shouldn't remove it now, those are scheduled to be removed in 4.1.

But I'm OK with just ignoring that for now, TBH. They will go away soon anyway.

[blueyed]

Cool, closing then.

[blueyed]

After all I think it is worth not having a bunch of warnings (that are expected).
I will look into it, but any pointers are appreciated of course - the one from Ronny seems good already.

[nicoddemus]

I think it is fine to ignore them by configuring filterwarnings in pytest.ini.

[Zac-HD]

Ignoring internal warnings in our own tests seems very risky to me - at some point we'll deprecate something without removing an internal use, and then all downstream test suites using -Werror will fail 😭

In Hypothesis we get around this with an explicit decorator for all tests that should raise deprecation warnings - it even errors if no warning is emitted.

IMO we should also test Pytest with all warnings as errors, except for jobs deliberately testing older versions of dependencies, to make this practical for downstream users.

[edmorley]

I'm seeing these warnings in our project's own pytest runs. I presume this is not expected?
https://travis-ci.org/mozilla/treeherder/jobs/442405171#L777

Our pytest config is here:
https://github.com/mozilla/treeherder/blob/97480c66c9700b77dae581205207802848bdea6c/setup.cfg#L32-L61

[nicoddemus]

Ignoring internal warnings in our own tests seems very risky to me

Oh I meant to ignore the warnings mentioned in this thread explicitly, not all internal warnings, which I agree would be very risky. 👍

IMO we should also test Pytest with all warnings as errors

We do that already 😉

https://github.com/pytest-dev/pytest/blob/e4871f7722d4254b28718e9c27b7e927ecf66191/tox.ini#L206-L220

[Zac-HD]

That only applies after pytest has finished the configure stage though, right? In the configure stage, _issue_config_warning downgrades warnings from errors to capturing them, even if we run with python -Werror -m pytest. IMO we should

• have a minimal dependency subset of the tests that we can run with Python-level warnings-as-errors
• issue a warning directly from _issue_config_warning if warnings have already been configured
• consider silencing warnings at the origin where we are using deprecated APIs for compatibility only

I'm poking at some of these now; I'll update with results when I have some.

---

diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py
index 6c4b921..e98d56a 100644
--- a/src/_pytest/warnings.py
+++ b/src/_pytest/warnings.py
@@ -165,8 +165,12 @@ def _issue_config_warning(warning, config):
     :param config:
     """
     with warnings.catch_warnings(record=True) as records:
-        warnings.simplefilter("always", type(warning))
+        # Set this warning to 'always', unless warnings have been configured
+        # at the interpreter level with e.g. -Werror or PYTHONWARNINGS=ignore
+        if sys.warnoptions:
+            warnings.simplefilter("always", type(warning))
         warnings.warn(warning, stacklevel=2)
-    config.hook.pytest_warning_captured.call_historic(
-        kwargs=dict(warning_message=records[0], when="config", item=None)
-    )
+    if records:
+        config.hook.pytest_warning_captured.call_historic(
+            kwargs=dict(warning_message=records[0], when="config", item=None)
+        )

Pretty sure this works but the setup to test it is awfully convoluted. I'll leave it there for now before it turns into a serious un-fun slog.

[nicoddemus]

I'm poking at some of these now; I'll update with results when I have some.

Awesome, thanks!

It is a little tricky to capture the warnings properly during pytest_configure because at that point the warnings plugin is not fully initialized, so the filter warnings filter is not materialized yet. But please share the results of your findings! 👍

@edmorley, you should not be seeing those warnings unless you or some plugins are accessing those properties. But we have realized that they might be triggered falsely in some test suites, and #4164 fixes this. 👍

[edmorley]

@edmorley, you should not be seeing those warnings unless you or some plugins are accessing those properties.

Thank you for the reply. The problem is that the current warning gives no indication as to what triggered the warning. For example:

$ python -3 -m pytest tests/ --runslow --ignore=tests/selenium/
/home/travis/venv/lib/python2.7/site-packages/_pytest/compat.py:329: RemovedInPytest4Warning: usage of Session.Class is deprecated, please use pytest.Class instead
  return getattr(object, name, default)

Perhaps this is another case of the warning's stacklevel needing to be adjusted?

[nicoddemus]

Perhaps this is another case of the warning's stacklevel needing to be adjusted?

Definitely. #4221 will also help here.

[nicoddemus]

Created #4236 to adjust the stack level.

[nicoddemus]

Fixed by #2701