[Development] Duplicated test data tags
Edward Welbourne
edward.welbourne at qt.io
Fri Oct 14 15:47:12 CEST 2022
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.
More information about the Development
mailing list