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 <macadder1@gmail.com>
(cherry picked from commit 0462dba7665450bdd0dc07e6a7e6ebe2805994a9)

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()

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
+ * on failure and the check for (QTest::runningTest() &&
- * qWaitFor(). (We no longer care about the bug in MSVC < 2017 that precluded
+ * QTest::currentTestFailed()) go beyond qWaitFor(). (We no longer care about
- * using qWaitFor() in the implementation here, see QTBUG-59096.)
+ * 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);

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;

tests/auto/testlib/selftests/expected_eventloop.junitxml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<testsuite name="tst_EventLoop" timestamp="@TEST_START_TIME@" hostname="@HOSTNAME@" tests="5" failures="3" errors="0" skipped="0" time="@TEST_DURATION@">
+<testsuite name="tst_EventLoop" timestamp="@TEST_START_TIME@" hostname="@HOSTNAME@" tests="5" failures="1" errors="0" skipped="1" time="@TEST_DURATION@">
   <properties>
     <property name="QTestVersion" value="@INSERT_QT_VERSION_HERE@"/>
     <property name="QtVersion" value="@INSERT_QT_VERSION_HERE@"/>
@@ -10,10 +10,8 @@
     <failure type="fail" message="Failing test should still clean up"/>
   </testcase>
   <testcase name="skip" classname="tst_EventLoop" time="@TEST_DURATION@">
-    <failure type="fail" message="&apos;!std::exchange(m_inTestFunction, true)&apos; returned FALSE. (Earlier test failed to clean up)"/>
+    <skipped message="Skipping test should still clean up"/>
-  </testcase>
-  <testcase name="pass" classname="tst_EventLoop" time="@TEST_DURATION@">
-    <failure type="fail" message="&apos;!std::exchange(m_inTestFunction, true)&apos; returned FALSE. (Earlier test failed to clean up)"/>
   </testcase>
+  <testcase name="pass" classname="tst_EventLoop" time="@TEST_DURATION@"/>
   <testcase name="cleanupTestCase" classname="tst_EventLoop" time="@TEST_DURATION@"/>
 </testsuite>

tests/auto/testlib/selftests/expected_eventloop.lightxml

@@ -11,27 +11,16 @@
     <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
       <Description><![CDATA[Failing test should still clean up]]></Description>
     </Incident>
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup())]]></Description>
-    </Incident>
     <Duration msecs="0"/>
   </TestFunction>
   <TestFunction name="skip">
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
+    <Incident type="skip" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['!std::exchange(m_inTestFunction, true)' returned FALSE. (Earlier test failed to clean up)]]></Description>
+      <Description><![CDATA[Skipping test should still clean up]]></Description>
-    </Incident>
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup())]]></Description>
     </Incident>
     <Duration msecs="0"/>
   </TestFunction>
   <TestFunction name="pass">
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
+    <Incident type="pass" file="" line="0" />
-      <Description><![CDATA['!std::exchange(m_inTestFunction, true)' returned FALSE. (Earlier test failed to clean up)]]></Description>
-    </Incident>
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup())]]></Description>
-    </Incident>
     <Duration msecs="0"/>
   </TestFunction>
   <TestFunction name="cleanupTestCase">

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()
+ok 3 - skip() # SKIP Skipping test should still clean up
-  ---
+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::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 5 - cleanupTestCase()
 1..5
 # tests 5
-# pass 2
+# pass 3
-# fail 3
+# fail 1

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[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[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[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']

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())
+SKIP   : tst_EventLoop::skip() Skipping test should still clean up
-   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())
    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 *********

tests/auto/testlib/selftests/expected_eventloop.xml

@@ -13,27 +13,16 @@
     <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
       <Description><![CDATA[Failing test should still clean up]]></Description>
     </Incident>
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup())]]></Description>
-    </Incident>
     <Duration msecs="0"/>
   </TestFunction>
   <TestFunction name="skip">
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
+    <Incident type="skip" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['!std::exchange(m_inTestFunction, true)' returned FALSE. (Earlier test failed to clean up)]]></Description>
+      <Description><![CDATA[Skipping test should still clean up]]></Description>
-    </Incident>
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup())]]></Description>
     </Incident>
     <Duration msecs="0"/>
   </TestFunction>
   <TestFunction name="pass">
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
+    <Incident type="pass" file="" line="0" />
-      <Description><![CDATA['!std::exchange(m_inTestFunction, true)' returned FALSE. (Earlier test failed to clean up)]]></Description>
-    </Incident>
-    <Incident type="fail" file="qtbase/tests/auto/testlib/selftests/eventloop/tst_eventloop.cpp" line="0">
-      <Description><![CDATA['loop.timeout()' returned FALSE. (QTestEventLoop exited prematurely in cleanup())]]></Description>
-    </Incident>
     <Duration msecs="0"/>
   </TestFunction>
   <TestFunction name="cleanupTestCase">