From 25695a4826aeb727d1a81cd506af77a75d02ccb0 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Sat, 9 Jul 2022 16:12:40 +0200 Subject: [PATCH] testlib: Don't print QCOMPARE values if they lack string representation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before 0681a2dd5a8095baddb5905fb21a58ce19b958c5, QCOMPARE'ing types for which no QTest::toString specialization exists did not output Actual and Expected lines on failure, as that would only print for both values (which then look like the same value, confusingly). Commit 0681a2dd5a8095baddb5905fb21a58ce19b958c5 changed that behavior, and started printing the confusing values. Take care of the logic in the formatFailMessage function: if both values are nullptr, then print only the variable names, but not the confusing text representation of the values. Remove dead and duplicated code related to the formatting logic, add a self-test function, and update the expected_cmptest files. Fixes: QTBUG-104867 Change-Id: I4be98e79f91196b14690a2cc0a68ffd50b431a45 Reviewed-by: Tor Arne Vestbø Reviewed-by: Thiago Macieira (cherry picked from commit 54b276be0b5885bbaee2c38f472eb39731fd684a) Reviewed-by: Qt Cherry-pick Bot --- src/testlib/qtestcase.h | 11 -- src/testlib/qtestresult.cpp | 60 +++++----- .../testlib/selftests/cmptest/tst_cmptest.cpp | 9 ++ .../selftests/expected_cmptest.junitxml | 8 +- .../selftests/expected_cmptest.lightxml | 8 ++ .../testlib/selftests/expected_cmptest.tap | 113 ++++++++++-------- .../selftests/expected_cmptest.teamcity | 3 + .../testlib/selftests/expected_cmptest.txt | 6 +- .../testlib/selftests/expected_cmptest.xml | 8 ++ 9 files changed, 133 insertions(+), 93 deletions(-) diff --git a/src/testlib/qtestcase.h b/src/testlib/qtestcase.h index 7db64af8ab1..0412f872f78 100644 --- a/src/testlib/qtestcase.h +++ b/src/testlib/qtestcase.h @@ -437,17 +437,6 @@ namespace QTest Q_TESTLIB_EXPORT QTestData &newRow(const char *dataTag); Q_TESTLIB_EXPORT QTestData &addRow(const char *format, ...) Q_ATTRIBUTE_FORMAT_PRINTF(1, 2); -#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) - // kept after adding implementation of out of paranoia: - template - inline bool qCompare(T const &t1, T const &t2, const char *actual, const char *expected, - const char *file, int line) - { - return compare_helper(t1 == t2, "Compared values are not the same", - toString(t1), toString(t2), actual, expected, file, line); - } -#endif - Q_TESTLIB_EXPORT bool qCompare(qfloat16 const &t1, qfloat16 const &t2, const char *actual, const char *expected, const char *file, int line); diff --git a/src/testlib/qtestresult.cpp b/src/testlib/qtestresult.cpp index 14d344f3818..556cafd4d45 100644 --- a/src/testlib/qtestresult.cpp +++ b/src/testlib/qtestresult.cpp @@ -321,29 +321,6 @@ static const char *rightArgNameForOp(QTest::ComparisonOperation op) return op == QTest::ComparisonOperation::CustomCompare ? "Expected " : "Right "; } -// Format failures using the toString() template -template -void formatFailMessage(char *msg, size_t maxMsgLen, - const char *failureMsg, - const Actual &val1, const Expected &val2, - const char *actual, const char *expected, - QTest::ComparisonOperation op) -{ - auto val1S = QTest::toString(val1); - auto val2S = QTest::toString(val2); - - size_t len1 = mbstowcs(nullptr, actual, maxMsgLen); // Last parameter is not ignored on QNX - size_t len2 = mbstowcs(nullptr, expected, maxMsgLen); // (result is never larger than this). - qsnprintf(msg, maxMsgLen, "%s\n %s(%s)%*s %s\n %s(%s)%*s %s", failureMsg, - leftArgNameForOp(op), actual, qMax(len1, len2) - len1 + 1, ":", - val1S ? val1S : "", - rightArgNameForOp(op), expected, qMax(len1, len2) - len2 + 1, ":", - val2S ? val2S : ""); - - delete [] val1S; - delete [] val2S; -} - // Overload to format failures for "const char *" - no need to strdup(). void formatFailMessage(char *msg, size_t maxMsgLen, const char *failureMsg, @@ -353,11 +330,38 @@ void formatFailMessage(char *msg, size_t maxMsgLen, { size_t len1 = mbstowcs(nullptr, actual, maxMsgLen); // Last parameter is not ignored on QNX size_t len2 = mbstowcs(nullptr, expected, maxMsgLen); // (result is never larger than this). - qsnprintf(msg, maxMsgLen, "%s\n %s(%s)%*s %s\n %s(%s)%*s %s", failureMsg, - leftArgNameForOp(op), actual, qMax(len1, len2) - len1 + 1, ":", - val1 ? val1 : "", - rightArgNameForOp(op), expected, qMax(len1, len2) - len2 + 1, ":", - val2 ? val2 : ""); + const int written = qsnprintf(msg, maxMsgLen, "%s\n", failureMsg); + msg += written; + maxMsgLen -= written; + + if (val1 || val2) { + qsnprintf(msg, maxMsgLen, " %s(%s)%*s %s\n %s(%s)%*s %s", + leftArgNameForOp(op), actual, qMax(len1, len2) - len1 + 1, ":", + val1 ? val1 : "", + rightArgNameForOp(op), expected, qMax(len1, len2) - len2 + 1, ":", + val2 ? val2 : ""); + } else { + // only print variable names if neither value can be represented as a string + qsnprintf(msg, maxMsgLen, " %s: %s\n %s: %s", + leftArgNameForOp(op), actual, rightArgNameForOp(op), expected); + } +} + +// Format failures using the toString() template +template +void formatFailMessage(char *msg, size_t maxMsgLen, + const char *failureMsg, + const Actual &val1, const Expected &val2, + const char *actual, const char *expected, + QTest::ComparisonOperation op) +{ + const char *val1S = QTest::toString(val1); + const char *val2S = QTest::toString(val2); + + formatFailMessage(msg, maxMsgLen, failureMsg, val1S, val2S, actual, expected, op); + + delete [] val1S; + delete [] val2S; } template diff --git a/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp b/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp index 5633a0252b9..5842d7b3b15 100644 --- a/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp +++ b/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp @@ -114,6 +114,7 @@ private slots: void compare_pointerfuncs(); void compare_tostring(); void compare_tostring_data(); + void compare_unknown(); void compareQObjects(); void compareQStringLists(); void compareQStringLists_data(); @@ -323,6 +324,14 @@ void tst_Cmptest::compare_tostring() QCOMPARE(actual, expected); } +void tst_Cmptest::compare_unknown() +{ + std::string a("a"); + std::string b("b"); + + QCOMPARE(a, b); +} + void tst_Cmptest::compareQStringLists_data() { QTest::addColumn("opA"); diff --git a/tests/auto/testlib/selftests/expected_cmptest.junitxml b/tests/auto/testlib/selftests/expected_cmptest.junitxml index 9509c067e2b..2c92cc66fd6 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.junitxml +++ b/tests/auto/testlib/selftests/expected_cmptest.junitxml @@ -1,5 +1,5 @@ - + @@ -78,6 +78,12 @@ Expected (expected): QVariant(PhonyClass,)]]> + + + + + + + + + + + )|n Expected (expected): QVariant(PhonyClass,)' flowId='tst_Cmptest'] ##teamcity[testFinished name='compare_tostring(both non-null user type)' flowId='tst_Cmptest'] +##teamcity[testStarted name='compare_unknown()' flowId='tst_Cmptest'] +##teamcity[testFailed name='compare_unknown()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)|]' details='Compared values are not the same|n Actual : a|n Expected : b' flowId='tst_Cmptest'] +##teamcity[testFinished name='compare_unknown()' flowId='tst_Cmptest'] ##teamcity[testStarted name='compareQObjects()' flowId='tst_Cmptest'] ##teamcity[testFailed name='compareQObjects()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)|]' details='Compared QObject pointers are not the same|n Actual (&object1): QObject/"object1"|n Expected (&object2): QObject/"object2"' flowId='tst_Cmptest'] ##teamcity[testFinished name='compareQObjects()' flowId='tst_Cmptest'] diff --git a/tests/auto/testlib/selftests/expected_cmptest.txt b/tests/auto/testlib/selftests/expected_cmptest.txt index 0fe70cdef64..993cbfa53b9 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.txt +++ b/tests/auto/testlib/selftests/expected_cmptest.txt @@ -51,6 +51,10 @@ FAIL! : tst_Cmptest::compare_tostring(both non-null user type) Compared values Actual (actual) : QVariant(PhonyClass,) Expected (expected): QVariant(PhonyClass,) Loc: [qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)] +FAIL! : tst_Cmptest::compare_unknown() Compared values are not the same + Actual : a + Expected : b + Loc: [qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)] FAIL! : tst_Cmptest::compareQObjects() Compared QObject pointers are not the same Actual (&object1): QObject/"object1" Expected (&object2): QObject/"object2" @@ -193,5 +197,5 @@ FAIL! : tst_Cmptest::tryVerify2() 'opaqueFunc() < 2' returned FALSE. (42) Loc: [qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)] PASS : tst_Cmptest::verifyExplicitOperatorBool() PASS : tst_Cmptest::cleanupTestCase() -Totals: 21 passed, 46 failed, 0 skipped, 0 blacklisted, 0ms +Totals: 21 passed, 47 failed, 0 skipped, 0 blacklisted, 0ms ********* Finished testing of tst_Cmptest ********* diff --git a/tests/auto/testlib/selftests/expected_cmptest.xml b/tests/auto/testlib/selftests/expected_cmptest.xml index 509dca88b7b..08c8d4694f9 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.xml +++ b/tests/auto/testlib/selftests/expected_cmptest.xml @@ -111,6 +111,14 @@ + + + + + +