D27910: Make sure warning output is enabled before testing if the correct warning is printed

[David Faure]

dfaure added a comment.


  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.

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D27910

To: sandsmark, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27910: Make sure warning output is enabled before testing if the correct warning is printed

[Martin Tobias Holmedahl Sandsmark]

sandsmark abandoned this revision.
sandsmark added a comment.


  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");`

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D27910

To: sandsmark, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27910: Make sure warning output is enabled before testing if the correct warning is printed

[David Faure]

dfaure added a comment.


  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.

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D27910

To: sandsmark, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27910: Make sure warning output is enabled before testing if the correct warning is printed

[David Faure]

dfaure added a comment.


  I suggest a better solution: QStandardPaths::setTestModeEnabled(true) in 
initTestCase().
  
  Then the user settings won't interfere with the unittest.

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D27910

To: sandsmark, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27910: Make sure warning output is enabled before testing if the correct warning is printed

[Aleix Pol Gonzalez]

apol added a comment.


  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/

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D27910

To: sandsmark, dfaure
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27910: Make sure warning output is enabled before testing if the correct warning is printed

[Martin Tobias Holmedahl Sandsmark]

sandsmark updated this revision to Diff 77165.
sandsmark added a comment.


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

REPOSITORY
  R243 KArchive

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27910?vs=77164=77165

REVISION DETAIL
  https://phabricator.kde.org/D27910

AFFECTED FILES
  autotests/karchivetest.cpp

To: sandsmark, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27910: Make sure warning output is enabled before testing if the correct warning is printed

[Martin Tobias Holmedahl Sandsmark]

sandsmark created this revision.
sandsmark added a reviewer: dfaure.
sandsmark added a project: Frameworks.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.
sandsmark requested review of this revision.

REVISION 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.

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D27910

AFFECTED FILES
  karchivetest.cpp

To: sandsmark, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns