From 10d88ca9db910467c5b60d59395054dd58b86df0 Mon Sep 17 00:00:00 2001 From: Vladimir Belyavsky Date: Fri, 17 Nov 2023 02:16:58 +0300 Subject: [PATCH] QTest: make failOnWarning() functional on temp objects destruction We need to be able to handle warnings that may occur when temporary objects, that were created in a test function, are destroyed. For example, now we miss all warnings that might be triggered from the object destructor, if the object's deletion was delayed (e.g. via deleteLater()). Also we miss all the warnings that were triggered on the test's cleanup() call. To fix this we need simply move QTestLog::clearFailOnWarnings() from QTestResult::finishedCurrentTestData() to the later stage, i.e. into QTestLog::clearCurrentTestState() which is actually called in appropriate time from QTestResult::finishedCurrentTestDataCleanup(). Same for QTestLog::clearIgnoreMessages(), since they are interrelated, so we need to clear them at the same time. We need this change for QML tests in particularly, to be able fail on warnings that might be triggered from Component.onDestruction() of some temporary test object. Pick-to: 6.5 Change-Id: I58a57691f20761619f56bd1bea3a862f2c26f569 Reviewed-by: Mitch Curtis Reviewed-by: Volker Hilsheimer (cherry picked from commit 18aa36cf878a52b8fe991392098e9445c3d3bfe3) Reviewed-by: Qt Cherry-pick Bot --- src/testlib/qtestlog.cpp | 3 ++- src/testlib/qtestresult.cpp | 12 +++++----- .../selftests/expected_warnings.junitxml | 7 +++++- .../selftests/expected_warnings.lightxml | 7 ++++++ .../testlib/selftests/expected_warnings.tap | 16 ++++++++++---- .../selftests/expected_warnings.teamcity | 3 +++ .../testlib/selftests/expected_warnings.txt | 5 ++++- .../testlib/selftests/expected_warnings.xml | 7 ++++++ .../selftests/warnings/tst_warnings.cpp | 22 +++++++++++++++++++ 9 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/testlib/qtestlog.cpp b/src/testlib/qtestlog.cpp index e01da379115..9fa165c11f9 100644 --- a/src/testlib/qtestlog.cpp +++ b/src/testlib/qtestlog.cpp @@ -318,7 +318,6 @@ void QTestLog::clearIgnoreMessages() QTest::IgnoreResultList::clearList(QTest::ignoreResultList); } - void QTestLog::clearFailOnWarnings() { QTest::failOnWarningList.clear(); @@ -326,6 +325,8 @@ void QTestLog::clearFailOnWarnings() void QTestLog::clearCurrentTestState() { + clearIgnoreMessages(); + clearFailOnWarnings(); QTest::currentTestState = QTest::Unresolved; } diff --git a/src/testlib/qtestresult.cpp b/src/testlib/qtestresult.cpp index 4f20404aa4f..093e9b58ef5 100644 --- a/src/testlib/qtestresult.cpp +++ b/src/testlib/qtestresult.cpp @@ -149,13 +149,6 @@ void QTestResult::finishedCurrentTestData() addFailure("QEXPECT_FAIL was called without any subsequent verification statements"); clearExpectFail(); - - if (!QTest::hasFailed() && QTestLog::unhandledIgnoreMessages()) { - QTestLog::printUnhandledIgnoreMessages(); - addFailure("Not all expected messages were received"); - } - QTestLog::clearIgnoreMessages(); - QTestLog::clearFailOnWarnings(); } /*! @@ -175,6 +168,11 @@ void QTestResult::finishedCurrentTestData() */ void QTestResult::finishedCurrentTestDataCleanup() { + if (!QTest::hasFailed() && QTestLog::unhandledIgnoreMessages()) { + QTestLog::printUnhandledIgnoreMessages(); + addFailure("Not all expected messages were received"); + } + // If the current test hasn't failed or been skipped, then it passes. if (!QTest::hasFailed() && !QTest::skipCurrentTest) { if (QTest::blacklistCurrentTest) diff --git a/tests/auto/testlib/selftests/expected_warnings.junitxml b/tests/auto/testlib/selftests/expected_warnings.junitxml index 1799be8279b..4fa8c88622d 100644 --- a/tests/auto/testlib/selftests/expected_warnings.junitxml +++ b/tests/auto/testlib/selftests/expected_warnings.junitxml @@ -1,5 +1,5 @@ - + @@ -107,5 +107,10 @@ + + + + + diff --git a/tests/auto/testlib/selftests/expected_warnings.lightxml b/tests/auto/testlib/selftests/expected_warnings.lightxml index 37070cad001..50b6c4bdb65 100644 --- a/tests/auto/testlib/selftests/expected_warnings.lightxml +++ b/tests/auto/testlib/selftests/expected_warnings.lightxml @@ -201,6 +201,13 @@ Ran out of cabbage!]]> + + + + + + diff --git a/tests/auto/testlib/selftests/expected_warnings.tap b/tests/auto/testlib/selftests/expected_warnings.tap index 9ea9b46bcbd..26675740640 100644 --- a/tests/auto/testlib/selftests/expected_warnings.tap +++ b/tests/auto/testlib/selftests/expected_warnings.tap @@ -159,8 +159,16 @@ Ran out of cabbage! ... ok 13 - testFailOnWarningsThenSkip() # SKIP My cabbage! :( ok 14 - testFailOnWarningsAndIgnoreWarnings() -ok 15 - cleanupTestCase() -1..15 -# tests 15 +not ok 15 - testFailOnTemporaryObjectDestruction() + --- + # Received a warning that resulted in a failure: +Running low on toothpaste! + at: tst_Warnings::testFailOnTemporaryObjectDestruction() (qtbase/tests/auto/testlib/selftests/warnings/tst_warnings.cpp:0) + file: qtbase/tests/auto/testlib/selftests/warnings/tst_warnings.cpp + line: 0 + ... +ok 16 - cleanupTestCase() +1..16 +# tests 16 # pass 5 -# fail 10 +# fail 11 diff --git a/tests/auto/testlib/selftests/expected_warnings.teamcity b/tests/auto/testlib/selftests/expected_warnings.teamcity index 67dd039ebfc..a4bd7139a67 100644 --- a/tests/auto/testlib/selftests/expected_warnings.teamcity +++ b/tests/auto/testlib/selftests/expected_warnings.teamcity @@ -57,6 +57,9 @@ ##teamcity[testFinished name='testFailOnWarningsThenSkip()' flowId='tst_Warnings'] ##teamcity[testStarted name='testFailOnWarningsAndIgnoreWarnings()' flowId='tst_Warnings'] ##teamcity[testFinished name='testFailOnWarningsAndIgnoreWarnings()' flowId='tst_Warnings'] +##teamcity[testStarted name='testFailOnTemporaryObjectDestruction()' flowId='tst_Warnings'] +##teamcity[testFailed name='testFailOnTemporaryObjectDestruction()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/warnings/tst_warnings.cpp(0)|]' details='Received a warning that resulted in a failure:|nRunning low on toothpaste!' flowId='tst_Warnings'] +##teamcity[testFinished name='testFailOnTemporaryObjectDestruction()' flowId='tst_Warnings'] ##teamcity[testStarted name='cleanupTestCase()' flowId='tst_Warnings'] ##teamcity[testFinished name='cleanupTestCase()' flowId='tst_Warnings'] ##teamcity[testSuiteFinished name='tst_Warnings' flowId='tst_Warnings'] diff --git a/tests/auto/testlib/selftests/expected_warnings.txt b/tests/auto/testlib/selftests/expected_warnings.txt index 5037eb87df3..72e2201f38d 100644 --- a/tests/auto/testlib/selftests/expected_warnings.txt +++ b/tests/auto/testlib/selftests/expected_warnings.txt @@ -69,6 +69,9 @@ Ran out of cabbage! SKIP : tst_Warnings::testFailOnWarningsThenSkip() My cabbage! :( Loc: [qtbase/tests/auto/testlib/selftests/warnings/tst_warnings.cpp(0)] PASS : tst_Warnings::testFailOnWarningsAndIgnoreWarnings() +FAIL! : tst_Warnings::testFailOnTemporaryObjectDestruction() Received a warning that resulted in a failure: +Running low on toothpaste! + Loc: [qtbase/tests/auto/testlib/selftests/warnings/tst_warnings.cpp(0)] PASS : tst_Warnings::cleanupTestCase() -Totals: 5 passed, 10 failed, 0 skipped, 0 blacklisted, 0ms +Totals: 5 passed, 11 failed, 0 skipped, 0 blacklisted, 0ms ********* Finished testing of tst_Warnings ********* diff --git a/tests/auto/testlib/selftests/expected_warnings.xml b/tests/auto/testlib/selftests/expected_warnings.xml index 8da8404a16d..808f59768c4 100644 --- a/tests/auto/testlib/selftests/expected_warnings.xml +++ b/tests/auto/testlib/selftests/expected_warnings.xml @@ -203,6 +203,13 @@ Ran out of cabbage!]]> + + + + + + diff --git a/tests/auto/testlib/selftests/warnings/tst_warnings.cpp b/tests/auto/testlib/selftests/warnings/tst_warnings.cpp index c113fdaf254..3dd9a7afe2b 100644 --- a/tests/auto/testlib/selftests/warnings/tst_warnings.cpp +++ b/tests/auto/testlib/selftests/warnings/tst_warnings.cpp @@ -27,6 +27,7 @@ private slots: void testFailOnWarningsThenSkip(); #endif void testFailOnWarningsAndIgnoreWarnings(); + void testFailOnTemporaryObjectDestruction(); }; void tst_Warnings::testWarnings() @@ -208,6 +209,27 @@ void tst_Warnings::testFailOnWarningsAndIgnoreWarnings() qWarning(warningStr); } +void tst_Warnings::testFailOnTemporaryObjectDestruction() +{ + QTest::failOnWarning("Running low on toothpaste!"); + QTest::ignoreMessage(QtWarningMsg, "Ran out of cabbage!"); + + class TestObject : public QObject + { + public: + ~TestObject() + { + // Shouldn't fail - ignored + qWarning("Ran out of cabbage!"); + // Should fail + qWarning("Running low on toothpaste!"); + } + }; + + QScopedPointer testObject(new TestObject); + QVERIFY(testObject); +} + QTEST_MAIN(tst_Warnings) #include "tst_warnings.moc"