Log capturing breaks existing logging tests

[The-Compiler]

#40 breaks an existing test in qutebrowser which uses a custom message handler which uses logging and pytest-capturelog.

I'm not sure whether this is an issue, or the kind of breakage which is okay when updating a minor version. If you think it should be fixed, probably the message handler should be restored/left untouched when qInstallMessageHandler returns something else than None (and a pytest warning shown?).

Another possibility would perhaps be to call the old message handler from the pytest-qt one, so both work?

[nicoddemus]

I think the solution is to simply disable the new feature, since they both seem to conflict:

[pytest]
addopts= --no-qt-log

I just tested this change in qutebrowser and it fixes that test. Another solution would be to rewrite the test using the new qtlog fixture.

Your suggestion about giving a warning if qInstallMessageHandler returns other than None seems good though, I will think about how to best display that to the user.

Thanks for checking out the release so quickly! 😄

[The-Compiler]

I looked at the test again now - what it actually tests is the filtering of irrelevant Qt messages inside qutebrowser.

qutebrowser redirects Qt warnings to a logger, and then has a custom LogFilter to filter harmless messages (others get displayed).

What that test does actually, is testing that filter. So I think there are two possible solutions:

• Log directly to the qt logging logger to test the filter instead of using qWarning.
• Add some kind of context manager to qtbot which temporarily restores the original handler and then in __exit__ sets the capturing handler again.

Of course I'd like to avoid using --no-qt-log, as I was the one requesting that feature in the first place 😉

[nicoddemus]

Hmm I like the context manager idea... I quickly tried this but the test still fails:

    def test_unfiltered(self, caplog):
        """Test a message which is not filtered."""
        from PyQt5.QtCore import qInstallMessageHandler
        try:
            old = qInstallMessageHandler(None)
            with log.hide_qt_warning("World", logger='qt-tests'):
                with caplog.atLevel(logging.WARNING, logger='qt-tests'):
                    qWarning("Hello World")
        finally:
            qInstallMessageHandler(old)
        assert len(caplog.records()) == 1
        record = caplog.records()[0]
        assert record.levelname == 'WARNING'
        assert record.message == "Hello World" 

You have something else in mind?

[The-Compiler]

I fixed the tests by logging directly to the logger which should get filtered instead of qWarning.

Though I still think the context manager thing would be useful - however, at least for my use case, it'd have to restore the original handler (i.e. item.qt_previous_handler) instead of None.

[nicoddemus]

Yeah, I like the context manager idea. Feel free to comment on #56. 😄