From a111c15c0a2b693bb7eeae95ecce187ee58a6a12 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Tue, 5 Jul 2022 19:27:10 +0200 Subject: [PATCH] Skip early return from test loops during cleanup() The QTRY_* macros and QTestEventLoop exit early if the test has resolved; however, in the cleanup phase of a test, even if the test has failed, these loops should continue as normal. [ChangeLog][QtTest] During the cleanup() phase of a test, the QTRY_* macros and QTestEventLoop now ignore the test resolution, in contrast to when they are used from the test itself, which (since 6.3.0) exits the loops early if the test has failed. Fixes: QTBUG-104441 Change-Id: I2673161967cbbc57815155af698a9338ab98a686 Reviewed-by: Jason McDonald (cherry picked from commit 0462dba7665450bdd0dc07e6a7e6ebe2805994a9) --- src/testlib/qtestcase.cpp | 14 ++++ src/testlib/qtestcase.h | 12 ++-- src/testlib/qtesteventloop.h | 2 +- .../selftests/expected_eventloop.junitxml | 8 +-- .../selftests/expected_eventloop.lightxml | 17 +---- .../testlib/selftests/expected_eventloop.tap | 66 ++----------------- .../selftests/expected_eventloop.teamcity | 9 +-- .../testlib/selftests/expected_eventloop.txt | 13 +--- .../testlib/selftests/expected_eventloop.xml | 17 +---- 9 files changed, 39 insertions(+), 119 deletions(-) diff --git a/src/testlib/qtestcase.cpp b/src/testlib/qtestcase.cpp index 494a96cd641..d9c7dffdc71 100644 --- a/src/testlib/qtestcase.cpp +++ b/src/testlib/qtestcase.cpp @@ -464,6 +464,7 @@ class WatchDog; static QObject *currentTestObject = nullptr; static QString mainSourcePath; +static bool inTestFunction = false; #if defined(Q_OS_MACOS) static IOPMAssertionID macPowerSavingDisabled = 0; @@ -1107,6 +1108,7 @@ void TestMethods::invokeTestOnData(int index) const /* Benchmarking: for each accumulation iteration*/ bool invokeOk; do { + QTest::inTestFunction = true; if (m_initMethod.isValid()) m_initMethod.invoke(QTest::currentTestObject, Qt::DirectConnection); @@ -1128,6 +1130,7 @@ void TestMethods::invokeTestOnData(int index) const invokeOk = false; } + QTest::inTestFunction = false; QTestResult::finishedCurrentTestData(); if (!initQuit) { @@ -2974,6 +2977,17 @@ bool QTest::currentTestFailed() return QTestResult::currentTestFailed(); } +/* + Returns \c true during the run of the test-function and its set-up. + + Used by the \c{QTRY_*} macros and \l QTestEventLoop to check whether to + return when QTest::currentTestFailed() is true. +*/ +bool QTest::runningTest() +{ + return QTest::inTestFunction; +} + /*! \internal */ QObject *QTest::testObject() diff --git a/src/testlib/qtestcase.h b/src/testlib/qtestcase.h index dd0599e1967..dc0c1b7da69 100644 --- a/src/testlib/qtestcase.h +++ b/src/testlib/qtestcase.h @@ -148,9 +148,10 @@ inline void useVerifyThrowsException() {} /* Ideally we would adapt qWaitFor(), or a variant on it, to implement roughly * what the following provides as QTRY_LOOP_IMPL(); however, for now, the * reporting of how much to increase the timeout to (if within a factor of two) - * on failure and the check for QTest::currentTestFailed() go beyond - * qWaitFor(). (We no longer care about the bug in MSVC < 2017 that precluded - * using qWaitFor() in the implementation here, see QTBUG-59096.) + * on failure and the check for (QTest::runningTest() && + * QTest::currentTestFailed()) go beyond qWaitFor(). (We no longer care about + * the bug in MSVC < 2017 that precluded using qWaitFor() in the implementation + * here, see QTBUG-59096.) */ // NB: not do {...} while (0) wrapped, as qt_test_i is accessed after it @@ -159,14 +160,14 @@ inline void useVerifyThrowsException() {} QTest::qWait(0); \ } \ int qt_test_i = 0; \ - for (; qt_test_i < timeoutValue && !QTest::currentTestFailed() \ + for (; qt_test_i < timeoutValue && !(QTest::runningTest() && QTest::currentTestFailed()) \ && !(expr); qt_test_i += step) { \ QTest::qWait(step); \ } // Ends in a for-block, so doesn't want a following semicolon. #define QTRY_TIMEOUT_DEBUG_IMPL(expr, timeoutValue, step) \ - if (!QTest::currentTestFailed() && !(expr)) { \ + if (!(QTest::runningTest() && QTest::currentTestFailed()) && !(expr)) { \ QTRY_LOOP_IMPL(expr, 2 * (timeoutValue), step) \ if ((expr)) { \ QFAIL(qPrintable(QTest::Internal::formatTryTimeoutDebugMessage(\ @@ -414,6 +415,7 @@ namespace QTest Q_TESTLIB_EXPORT const char *currentTestFunction(); Q_TESTLIB_EXPORT const char *currentDataTag(); Q_TESTLIB_EXPORT bool currentTestFailed(); + Q_TESTLIB_EXPORT bool runningTest(); // Internal, for use by macros and QTestEventLoop. Q_TESTLIB_EXPORT Qt::Key asciiToKey(char ascii); Q_TESTLIB_EXPORT char keyToAscii(Qt::Key key); diff --git a/src/testlib/qtesteventloop.h b/src/testlib/qtesteventloop.h index 7a04c82da3f..a87426f8f07 100644 --- a/src/testlib/qtesteventloop.h +++ b/src/testlib/qtesteventloop.h @@ -60,7 +60,7 @@ inline void QTestEventLoop::enterLoopMSecs(int ms) Q_ASSERT(!loop); _timeout = false; - if (QTest::currentTestFailed()) + if (QTest::runningTest() && QTest::currentTestFailed()) return; QEventLoop l; diff --git a/tests/auto/testlib/selftests/expected_eventloop.junitxml b/tests/auto/testlib/selftests/expected_eventloop.junitxml index 15a21427c83..566bc1605ed 100644 --- a/tests/auto/testlib/selftests/expected_eventloop.junitxml +++ b/tests/auto/testlib/selftests/expected_eventloop.junitxml @@ -1,5 +1,5 @@ - + @@ -10,10 +10,8 @@ - - - - + + diff --git a/tests/auto/testlib/selftests/expected_eventloop.lightxml b/tests/auto/testlib/selftests/expected_eventloop.lightxml index 0d67d73f752..40880fde013 100644 --- a/tests/auto/testlib/selftests/expected_eventloop.lightxml +++ b/tests/auto/testlib/selftests/expected_eventloop.lightxml @@ -11,27 +11,16 @@ - - - - - - - - + + - - - - - - + diff --git a/tests/auto/testlib/selftests/expected_eventloop.tap b/tests/auto/testlib/selftests/expected_eventloop.tap index 85f48a13a16..496a6e636a9 100644 --- a/tests/auto/testlib/selftests/expected_eventloop.tap +++ b/tests/auto/testlib/selftests/expected_eventloop.tap @@ -8,68 +8,10 @@ not ok 2 - fail() file: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp line: 0 ... -not ok 2 - fail() - --- - type: QVERIFY - message: ) - wanted: true (loop.timeout()) - found: false (loop.timeout()) - expected: true (loop.timeout()) - actual: false (loop.timeout()) - at: tst_EventLoop::fail() (qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp:0) - file: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp - line: 0 - ... -not ok 3 - skip() - --- - type: QVERIFY - message: Earlier test failed to clean up - wanted: true (!std::exchange(m_inTestFunction, true)) - found: false (!std::exchange(m_inTestFunction, true)) - expected: true (!std::exchange(m_inTestFunction, true)) - actual: false (!std::exchange(m_inTestFunction, true)) - at: tst_EventLoop::skip() (qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp:0) - file: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp - line: 0 - ... -not ok 3 - skip() - --- - type: QVERIFY - message: ) - wanted: true (loop.timeout()) - found: false (loop.timeout()) - expected: true (loop.timeout()) - actual: false (loop.timeout()) - at: tst_EventLoop::skip() (qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp:0) - file: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp - line: 0 - ... -not ok 4 - pass() - --- - type: QVERIFY - message: Earlier test failed to clean up - wanted: true (!std::exchange(m_inTestFunction, true)) - found: false (!std::exchange(m_inTestFunction, true)) - expected: true (!std::exchange(m_inTestFunction, true)) - actual: false (!std::exchange(m_inTestFunction, true)) - at: tst_EventLoop::pass() (qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp:0) - file: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp - line: 0 - ... -not ok 4 - pass() - --- - type: QVERIFY - message: ) - wanted: true (loop.timeout()) - found: false (loop.timeout()) - expected: true (loop.timeout()) - actual: false (loop.timeout()) - at: tst_EventLoop::pass() (qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp:0) - file: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp - line: 0 - ... +ok 3 - skip() # SKIP Skipping test should still clean up +ok 4 - pass() ok 5 - cleanupTestCase() 1..5 # tests 5 -# pass 2 -# fail 3 +# pass 3 +# fail 1 diff --git a/tests/auto/testlib/selftests/expected_eventloop.teamcity b/tests/auto/testlib/selftests/expected_eventloop.teamcity index 4c832be52f7..a293a201357 100644 --- a/tests/auto/testlib/selftests/expected_eventloop.teamcity +++ b/tests/auto/testlib/selftests/expected_eventloop.teamcity @@ -4,17 +4,10 @@ ##teamcity[testStarted name='fail()' flowId='tst_EventLoop'] ##teamcity[testFailed name='fail()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' details='Failing test should still clean up' flowId='tst_EventLoop'] ##teamcity[testFinished name='fail()' flowId='tst_EventLoop'] -##teamcity[testFailed name='fail()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' details='|'loop.timeout()|' returned FALSE. (QTestEventLoop exited prematurely in cleanup())' flowId='tst_EventLoop'] -##teamcity[testFinished name='fail()' flowId='tst_EventLoop'] ##teamcity[testStarted name='skip()' flowId='tst_EventLoop'] -##teamcity[testFailed name='skip()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' details='|'!std::exchange(m_inTestFunction, true)|' returned FALSE. (Earlier test failed to clean up)' flowId='tst_EventLoop'] -##teamcity[testFinished name='skip()' flowId='tst_EventLoop'] -##teamcity[testFailed name='skip()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' details='|'loop.timeout()|' returned FALSE. (QTestEventLoop exited prematurely in cleanup())' flowId='tst_EventLoop'] +##teamcity[testIgnored name='skip()' message='Skipping test should still clean up |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' flowId='tst_EventLoop'] ##teamcity[testFinished name='skip()' flowId='tst_EventLoop'] ##teamcity[testStarted name='pass()' flowId='tst_EventLoop'] -##teamcity[testFailed name='pass()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' details='|'!std::exchange(m_inTestFunction, true)|' returned FALSE. (Earlier test failed to clean up)' flowId='tst_EventLoop'] -##teamcity[testFinished name='pass()' flowId='tst_EventLoop'] -##teamcity[testFailed name='pass()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)|]' details='|'loop.timeout()|' returned FALSE. (QTestEventLoop exited prematurely in cleanup())' flowId='tst_EventLoop'] ##teamcity[testFinished name='pass()' flowId='tst_EventLoop'] ##teamcity[testStarted name='cleanupTestCase()' flowId='tst_EventLoop'] ##teamcity[testFinished name='cleanupTestCase()' flowId='tst_EventLoop'] diff --git a/tests/auto/testlib/selftests/expected_eventloop.txt b/tests/auto/testlib/selftests/expected_eventloop.txt index b614c876c6d..548ef393f6d 100644 --- a/tests/auto/testlib/selftests/expected_eventloop.txt +++ b/tests/auto/testlib/selftests/expected_eventloop.txt @@ -3,16 +3,9 @@ Config: Using QtTest library PASS : tst_EventLoop::initTestCase() FAIL! : tst_EventLoop::fail() Failing test should still clean up Loc: [qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)] -FAIL! : tst_EventLoop::fail() 'loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup()) - Loc: [qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)] -FAIL! : tst_EventLoop::skip() '!std::exchange(m_inTestFunction, true)' returned FALSE. (Earlier test failed to clean up) - Loc: [qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)] -FAIL! : tst_EventLoop::skip() 'loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup()) - Loc: [qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)] -FAIL! : tst_EventLoop::pass() '!std::exchange(m_inTestFunction, true)' returned FALSE. (Earlier test failed to clean up) - Loc: [qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)] -FAIL! : tst_EventLoop::pass() 'loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup()) +SKIP : tst_EventLoop::skip() Skipping test should still clean up Loc: [qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp(0)] +PASS : tst_EventLoop::pass() PASS : tst_EventLoop::cleanupTestCase() -Totals: 2 passed, 3 failed, 0 skipped, 0 blacklisted, 0ms +Totals: 3 passed, 1 failed, 1 skipped, 0 blacklisted, 0ms ********* Finished testing of tst_EventLoop ********* diff --git a/tests/auto/testlib/selftests/expected_eventloop.xml b/tests/auto/testlib/selftests/expected_eventloop.xml index 2613a734494..f9d94756668 100644 --- a/tests/auto/testlib/selftests/expected_eventloop.xml +++ b/tests/auto/testlib/selftests/expected_eventloop.xml @@ -13,27 +13,16 @@ - - - - - - - - + + - - - - - - +