Milian Wolff (Friday, 14 October 2022 3:00 AM) wrote:
>> >> I have many times accidentally written bogus code that duplicated the
>> >> tags.  Getting a warning is useful, so thanks for working on that!
>> >> 
>> >> But we won't easily spot these in the thousands of lines of outputs a
>> >> large test suite is generating.

On Freitag, 14. Oktober 2022 10:55:54 CEST Edward Welbourne wrote:
>> Indeed, that strikes me as eminently plausible.
>> Especially as one of the things I've been picking up on (and I've fixed
>> various of them, at least in QtCore) as a result of chasing these down
>> is that we have quite a few tests with QWARN messages, that really
>> should be anticipated (so that the test fails if the warning isn't
>> produced; and so that the test log isn't cluttered with the warnings)
>> using QTest::ignoreWarning().
>> 
>> I should also take this opportunity to encourage all developers to watch
>> out for QWARN messages in test output: if it's unexpected, it may be the
>> symptom of a lurking bug; otherwise, its presence should be tested for,
>> see preceding.  If the code under test fails to produce a warning that
>> was expected, that's an issue the test should catch.

>> >> At the very least I would suggest something akin to QT_FATAL_WARNINGS
>> >> that can be set to more easily spot faulty client code.
>> 
>> Perhaps, given the above, we should just encourage developers to use
>> QT_FATAL_WARNINGS more often ?
>> 
>> I do note, however, that it seems not to have been documented; I shall
>> add a doc update to my branch.

Milian Wolff (Friday, October 14, 2022 12:29) replied:
> That is sadly not an option in many situations. The silencing of the warnings 
> does not influence the fatal-abort to my knowledge? See 
> `QMessageLogger::warning`:
> 
> ```
> void QMessageLogger::warning(const char *msg, ...) const
> {
>     va_list ap;
>     va_start(ap, msg); // use variable arg list
>     const QString message = qt_message(QtWarningMsg, context, msg, ap);
>     va_end(ap);
>
>     if (isFatal(QtWarningMsg))
>         qt_message_fatal(QtWarningMsg, context, message);
> }
> ```

That sounds like something we should look into changing.  It runs
counter to what QTest::ignoreWarnings() tells its user; they may
reasonably expect setting the environment variable to catch only
warnings they haven't suppressed.  However, that's very much within
QMessageLogger code, not under QTest's control :-(
QTBUG-107659

> So while `qt_message` is intercepted, the `qt_message_fatal` is not which 
> then 
> would kill the app even for "expected" warnings.

:-(

> What's worse, there are many warnings where you don't know how many of them 
> will occur. We e.g. run our unit test suite using the offscreen platform 
> plugin and are drowning in warnings such as:
>
> ```
> This plugin does not support setParent!
> ```
>
> And so far I could not find a reliable way to prevent this.

QTBUG-107660

> Similarly, we need a custom OpenGL profile which always triggers this warning 
> when Qt WebEngine is also used:
>
> ```
> An OpenGL Core Profile was requested, but it is not supported on the current 
> platform. Falling back to a non-Core profile. Note that this might cause 
> rendering issues.
> ```
>
> Meaning: there are sadly some situations where warnings are emitted that you 
> do not have control over. Meaning, Qt_FATAL_WARNINGS is sadly not an option 
> today :(

Sounds painful.
Thanks for the dose of real-world practicality.  I'll need to give
further thought to how to make the new warnings "enforceable"
(without enforcing them all the time).

        Eddy.
_______________________________________________
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

Reply via email to