Allow 'unknown config key' warnings to be filtered out

[nicoddemus]

Delay issuing unknown config key warnings until collection is done, which allows users to filter it out using filterwarnings.

It also combines all missing keys into a single warning, instead of issuing a separate warning, but I'm not sure that's a good idea or not (this is a separate commit so it is easy to take it out).

Fix #7620

[bluetech]

until collection is done

Why is this the correct time?

[nicoddemus]

Why is this the correct time?

Is just better than was before, at configure time, because at that time the warnings module wasn't configured yet. 😁

[bluetech]

Regarding Combine multiple 'unknown config key' warnings into one, maybe we actually want to keep them separate so a warning about a particular ini can be filtered, but not others?

---

Regarding Unknown ini key warnings is delayed so it can be filtered out:

First, are we even sure we want such warnings to appear only in the summary view? It seems perhaps more natural to show at the start already, and keep the summary warnings for collection/test warnings. But actually we seem to have precedence for putting all warnings there, so I guess the answer to this is "yes".

Second, the code used _issue_warning_captured, which is intended for use during the configure phase, but that has a problem that filterwarnings doesn't work for it, so to fix this you moved to issue the warnings at a later phase which is handled by the warnings plugin. But wouldn't it be better to fix the issue in _issue_warning_captured directly? This way all the other users of _issue_warning_captured will benefit as well.

Looks like what it needs is to harmonize the catch_warnings() call in _issue_warning_captured with the one in catch_warnings_for_item, at least the relevant parts, e.g.

diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py
index 0604aa60b..79cf547e7 100644
--- a/src/_pytest/warnings.py
+++ b/src/_pytest/warnings.py
@@ -191,7 +191,9 @@ def pytest_sessionfinish(session: Session) -> Generator[None, None, None]:
         yield
 
 
-def _issue_warning_captured(warning: Warning, hook, stacklevel: int) -> None:
+def _issue_warning_captured(
+    config: Config, warning: Warning, hook, stacklevel: int
+) -> None:
     """A function that should be used instead of calling ``warnings.warn``
     directly when we are in the "configure" stage.
 
@@ -205,6 +207,16 @@ def _issue_warning_captured(warning: Warning, hook, stacklevel: int) -> None:
     """
     with warnings.catch_warnings(record=True) as records:
         warnings.simplefilter("always", type(warning))
+
+        # Make -W/filterwarnings apply to these warnings as well.
+        # This code is the same as in catch_warnings_for_item().
+        cmdline_filters = config.getoption("pythonwarnings") or []
+        inifilters = config.getini("filterwarnings")
+        for arg in inifilters:
+            warnings.filterwarnings(*_parse_filter(arg, escape=False))
+        for arg in cmdline_filters:
+            warnings.filterwarnings(*_parse_filter(arg, escape=True))
+
         warnings.warn(warning, stacklevel=stacklevel)
     frame = sys._getframe(stacklevel - 1)
     location = frame.f_code.co_filename, frame.f_lineno, frame.f_code.co_name

This is untested and all of the _issue_warning_captured callers need to be fixed to also pass config, but I think it should work.

[nicoddemus]

Regarding Combine multiple 'unknown config key' warnings into one, maybe we actually want to keep them separate so a warning about a particular ini can be filtered, but not others?

Yep, I realized that right after writing the description, so I decided to keep the commits separate to be able to pluck them out. I agree, will do it. 👍

But wouldn't it be better to fix the issue in _issue_warning_captured directly? This way all the other users of _issue_warning_captured will benefit as well.

Oh you are definitely correct in your reasoning!

The solution you propose is on point, however I'm not sure it will work (see #2891 (comment)). However that was a long time ago, and deserves to be reviewed if that's still the case.

(Also noticed this might be related to fixing #6681).

I will give this another spin, thanks a lot @bluetech!

[nicoddemus]

Superseded by #7700