Differential D27910

Make sure warning output is enabled before testing if the correct warning is printed
AbandonedPublic
Actions

Authored by sandsmark on Mar 7 2020, 12:58 PM.

Details

Reviewers
dfaure
Summary

A couple of the tests check for specific warning messages, so we need to ensure that the required logging category is enabled.

Test Plan

Tests fail before this patch if turning off all logging for karchive with e. g. kdebugsettings, pass afterwards.

Diff Detail

Repository
R243 KArchive
Lint
Lint Skipped
Unit
Unit Tests Skipped
sandsmark created this revision.Mar 7 2020, 12:58 PM
Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptMar 7 2020, 12:58 PM
sandsmark requested review of this revision.Mar 7 2020, 12:58 PM
sandsmark updated this revision to Diff 77165.

Forgot that differential doesn't like patches created when git has noprefix turned on.

apol added a subscriber: apol.Mar 9 2020, 2:02 AM

Is it passing on the CI because somehow we enable warnings on there?
https://build.kde.org/job/Frameworks/job/karchive/job/kf5-qt5%20SUSEQt5.13/60/testReport/

dfaure added a comment.Mar 11 2020, 2:40 AM

I suggest a better solution: QStandardPaths::setTestModeEnabled(true) in initTestCase().

Then the user settings won't interfere with the unittest.

dfaure added a comment.Mar 21 2020, 6:09 PM

Note that in both cases the env var QT_LOGGING_RULES would break the unittest anyway... so yeah it's only about qtlogging.ini which we can easily skip with test mode.

Here's my suggested fix: D28189

@apol Warnings are enabled by default, for all categories, the way we define them.

sandsmark abandoned this revision.Apr 7 2020, 1:03 PM
In D27910#632041, @dfaure wrote:

Note that in both cases the env var QT_LOGGING_RULES would break the unittest anyway... so yeah it's only about qtlogging.ini which we can easily skip with test mode.

Yeah, the issue for me was qtlogging.ini, but it's not that far fetched that the user has set QT_LOGGING_RULES manually. Because it's a bit surprising and not obvious why this fails, maybe we could check if the warnings are disabled (e. g. if (!KArchiveLog().isWarningEnabled()) { qWarning()<< "Warnings disabled for" << KArchiveLog().categoryName() << ", the test will probably fail"; }), if not outright calling qunsetenv("QT_LOGGING_RULES");

dfaure added a comment.Apr 7 2020, 1:17 PM

I like your first suggestion.

I don't like the second one, because QT_LOGGING_RULES is used (by at least CI and myself) to enable *more* debug output. unsetenv would break that.

Revision Contents
Changeset List

PathPackages
Mautotests/karchivetest.cpp (6 lines)

Diff 77165

View Options

autotests/karchivetest.cpp

Loading...