Fix the skip-check in TestMethods::invokeTest()

TestMethods::invokeTest() has an outer loop on global data (albeit
with a comment that said otherwise). On its first cycle, we run the
test function's *_data() method, if it has one; there is an inner loop
on the rows this created. If the *_data() QSKIP()s, we need to skip
the whole test; otherwise, a QSKIP() in one sub-test should not lead
to skipping the remaining sub-tests.

Moved the check for *_data() QSKIP()ping to right after *_data()
returns, inside the "first global cycle" block that runs it.
Previously, this check was done before entering the loop on local data
rows, but outside that "first global cycle" block: consequently, later
global cycles would fall foul of this check (even though the *_data()
hasn't been run in this cycle, much less QSKIP()ped in it) if the last
sub-test of the previous global cycle had QSKIP()ped.

When running a single test for one specific data row, if the test's
*_data() QSKIP()ped, this misplaced check would also have lead to a
misleading "Unknown testdata" warning.

Changed testlib/selftests' tst_globaldata::skipSingle() to trigger the
bug (by having its last local row of first global row skip, which
caused the second global row to be omitted) to verify this is also
fixed; and amended one of its comments to reflect what's now to be
expected. Updated the test's expected output files.

Task-number: QTBUG-61774
Change-Id: I99596b595c6d1184038f23383844c6ff51a0cd91
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@qt.io>

1 files changed, 36 insertions, 36 deletions

diff --git a/src/testlib/qtestcase.cpp b/src/testlib/qtestcase.cpp
index d9f8dd7805..a202add2ae 100644
--- a/src/testlib/qtestcase.cpp
+++ b/src/testlib/qtestcase.cpp
@@ -1095,7 +1095,7 @@ bool TestMethods::invokeTest(int index, const char *data, WatchDog *watchDog) co
     const int globalDataCount = gTable->dataCount();
     int curGlobalDataIndex = 0;
 
-    /* For each test function that has a *_data() table/function, do: */
+    /* For each entry in the global data table, do: */
     do {
         if (!gTable->isEmpty())
             QTestResult::setCurrentGlobalTestData(gTable->testData(curGlobalDataIndex));
@@ -1103,50 +1103,50 @@ bool TestMethods::invokeTest(int index, const char *data, WatchDog *watchDog) co
         if (curGlobalDataIndex == 0) {
             qsnprintf(member, 512, "%s_data()", name.constData());
             invokeMethod(QTest::currentTestObject, member);
+            if (QTestResult::skipCurrentTest())
+                break;
         }
 
         bool foundFunction = false;
-        if (!QTestResult::skipCurrentTest()) {
-            int curDataIndex = 0;
-            const int dataCount = table.dataCount();
-
-            // Data tag requested but none available?
-            if (data && !dataCount) {
-                // Let empty data tag through.
-                if (!*data)
-                    data = 0;
-                else {
-                    fprintf(stderr, "Unknown testdata for function %s(): '%s'\n", name.constData(), data);
-                    fprintf(stderr, "Function has no testdata.\n");
-                    return false;
-                }
+        int curDataIndex = 0;
+        const int dataCount = table.dataCount();
+
+        // Data tag requested but none available?
+        if (data && !dataCount) {
+            // Let empty data tag through.
+            if (!*data)
+                data = 0;
+            else {
+                fprintf(stderr, "Unknown testdata for function %s(): '%s'\n", name.constData(), data);
+                fprintf(stderr, "Function has no testdata.\n");
+                return false;
             }
+        }
 
-            /* For each entry in the data table, do: */
-            do {
-                QTestResult::setSkipCurrentTest(false);
-                QTestResult::setBlacklistCurrentTest(false);
-                if (!data || !qstrcmp(data, table.testData(curDataIndex)->dataTag())) {
-                    foundFunction = true;
+        /* For each entry in this test's data table, do: */
+        do {
+            QTestResult::setSkipCurrentTest(false);
+            QTestResult::setBlacklistCurrentTest(false);
+            if (!data || !qstrcmp(data, table.testData(curDataIndex)->dataTag())) {
+                foundFunction = true;
 
-                    QTestPrivate::checkBlackLists(name.constData(), dataCount ? table.testData(curDataIndex)->dataTag() : 0);
+                QTestPrivate::checkBlackLists(name.constData(), dataCount ? table.testData(curDataIndex)->dataTag() : 0);
 
-                    QTestDataSetter s(curDataIndex >= dataCount ? static_cast<QTestData *>(0)
-                                                      : table.testData(curDataIndex));
+                QTestDataSetter s(curDataIndex >= dataCount ? static_cast<QTestData *>(0)
+                                  : table.testData(curDataIndex));
 
-                    QTestPrivate::qtestMouseButtons = Qt::NoButton;
-                    if (watchDog)
-                        watchDog->beginTest();
-                    invokeTestOnData(index);
-                    if (watchDog)
-                        watchDog->testFinished();
+                QTestPrivate::qtestMouseButtons = Qt::NoButton;
+                if (watchDog)
+                    watchDog->beginTest();
+                invokeTestOnData(index);
+                if (watchDog)
+                    watchDog->testFinished();
 
-                    if (data)
-                        break;
-                }
-                ++curDataIndex;
-            } while (curDataIndex < dataCount);
-        }
+                if (data)
+                    break;
+            }
+            ++curDataIndex;
+        } while (curDataIndex < dataCount);
 
         if (data && !foundFunction) {
             fprintf(stderr, "Unknown testdata for function %s: '%s()'\n", name.constData(), data);