From d81b065bb3cea85a98f2299354d05ce440032f1f Mon Sep 17 00:00:00 2001 From: Jason McDonald Date: Wed, 29 Feb 2012 15:43:42 +1000 Subject: [PATCH] testlib: Improve verbose and XPASS output Previously, verbose (-v2) and XPASS test output showed all QCOMPAREs as "COMPARE()", making it impossible to see what was compared and difficult to match the output to the source of a test containing many calls to QCOMPARE. This commit changes testlib's internal compare_helper API so that string representations of the compared expressions are always passed to QTestResult::compare() when available, and can thus be shown in the verbose and XPASS output. The XPASS output has also been changed to state explicitly that the comparison succeeded unexpectedly, bringing it in line with the XPASS output resulting from a call to QVERIFY. This commit also changes all calls to compare_helper() to call the eight-argument version of the function, which simplifies much of the calling code. The now obsolete four-argument version of compare_helper() has been changed to output a warning that it is obsolete. It will be removed once other modules have had some time to catch up. The improved XPASS and verbose output is demonstrated by the expectfail and verbose2 selftests. Change-Id: I8baa46d5dd30e6c43b26f366c34dc5b64aab5f7c Reviewed-by: Rohan McGovern --- src/testlib/qtest.h | 2 +- src/testlib/qtest_gui.h | 20 ++++----- src/testlib/qtestcase.cpp | 43 ++++++++++++------- src/testlib/qtestcase.h | 16 +++---- src/testlib/qtestresult.cpp | 34 +++++++++++---- src/testlib/qtestresult_p.h | 6 ++- .../selftests/expected_expectfail.lightxml | 2 +- .../testlib/selftests/expected_expectfail.txt | 2 +- .../testlib/selftests/expected_expectfail.xml | 2 +- .../selftests/expected_expectfail.xunitxml | 2 +- .../selftests/expected_verbose2.lightxml | 12 +++--- .../testlib/selftests/expected_verbose2.txt | 12 +++--- .../testlib/selftests/expected_verbose2.xml | 12 +++--- .../selftests/expected_verbose2.xunitxml | 24 +++++------ .../auto/testlib/selftests/tst_selftests.cpp | 2 +- 15 files changed, 108 insertions(+), 83 deletions(-) diff --git a/src/testlib/qtest.h b/src/testlib/qtest.h index 32b2f0ca9c6..477d344d718 100644 --- a/src/testlib/qtest.h +++ b/src/testlib/qtest.h @@ -210,7 +210,7 @@ inline bool qCompare(QStringList const &t1, QStringList const &t2, isOk = false; } } - return compare_helper(isOk, msg, file, line); + return compare_helper(isOk, msg, 0, 0, actual, expected, file, line); } template diff --git a/src/testlib/qtest_gui.h b/src/testlib/qtest_gui.h index d39e943926f..3067c35e026 100644 --- a/src/testlib/qtest_gui.h +++ b/src/testlib/qtest_gui.h @@ -102,29 +102,27 @@ inline bool qCompare(QImage const &t1, QImage const &t2, qsnprintf(msg, 1024, "Compared QImages differ.\n" " Actual (%s).isNull() : %d\n" " Expected (%s).isNull(): %d", actual, t1Null, expected, t2Null); - return compare_helper(false, msg, file, line); + return compare_helper(false, msg, 0, 0, actual, expected, file, line); } if (t1Null && t2Null) - return compare_helper(true, "COMPARE()", file, line); + return compare_helper(true, 0, 0, 0, actual, expected, file, line); if (t1.width() != t2.width() || t2.height() != t2.height()) { qsnprintf(msg, 1024, "Compared QImages differ in size.\n" " Actual (%s) : %dx%d\n" " Expected (%s): %dx%d", actual, t1.width(), t1.height(), expected, t2.width(), t2.height()); - return compare_helper(false, msg, file, line); + return compare_helper(false, msg, 0, 0, actual, expected, file, line); } if (t1.format() != t2.format()) { qsnprintf(msg, 1024, "Compared QImages differ in format.\n" " Actual (%s) : %d\n" " Expected (%s): %d", actual, t1.format(), expected, t2.format()); - return compare_helper(false, msg, file, line); + return compare_helper(false, msg, 0, 0, actual, expected, file, line); } - return (t1 == t2) - ? compare_helper(true, "COMPARE()", file, line) - : compare_helper(false, "Compared values are not the same", - toString(t1), toString(t2), actual, expected, file, line); + return compare_helper(t1 == t2, "Compared values are not the same", + toString(t1), toString(t2), actual, expected, file, line); } #ifndef QTEST_NO_SPECIALIZATIONS @@ -141,17 +139,17 @@ inline bool qCompare(QPixmap const &t1, QPixmap const &t2, const char *actual, c qsnprintf(msg, 1024, "Compared QPixmaps differ.\n" " Actual (%s).isNull() : %d\n" " Expected (%s).isNull(): %d", actual, t1Null, expected, t2Null); - return compare_helper(false, msg, file, line); + return compare_helper(false, msg, 0, 0, actual, expected, file, line); } if (t1Null && t2Null) - return compare_helper(true, "COMPARE()", file, line); + return compare_helper(true, 0, 0, 0, actual, expected, file, line); if (t1.width() != t2.width() || t2.height() != t2.height()) { qsnprintf(msg, 1024, "Compared QPixmaps differ in size.\n" " Actual (%s) : %dx%d\n" " Expected (%s): %dx%d", actual, t1.width(), t1.height(), expected, t2.width(), t2.height()); - return compare_helper(false, msg, file, line); + return compare_helper(false, msg, 0, 0, actual, expected, file, line); } return qCompare(t1.toImage(), t2.toImage(), actual, expected, file, line); } diff --git a/src/testlib/qtestcase.cpp b/src/testlib/qtestcase.cpp index ace080ef942..001a14a1213 100644 --- a/src/testlib/qtestcase.cpp +++ b/src/testlib/qtestcase.cpp @@ -2412,15 +2412,34 @@ QObject *QTest::testObject() */ bool QTest::compare_helper(bool success, const char *msg, const char *file, int line) { + static bool warned = false; + if (!warned) { + warned = true; + QTest::qWarn("QTest::compare_helper(bool, const char *, const char *, int) is obsolete " + "and will soon be removed. Please update your code to use the other " + "version of this function."); + } + return QTestResult::compare(success, msg, file, line); } /*! \internal + This function is called by various specializations of QTest::qCompare + to decide whether to report a failure and to produce verbose test output. + + The failureMsg parameter can be null, in which case a default message + will be output if the compare fails. If the compare succeeds, failureMsg + will not be output. + + If the caller has already passed a failure message showing the compared + values, or if those values cannot be stringified, val1 and val2 can be null. */ -bool QTest::compare_helper(bool success, const char *msg, char *val1, char *val2, - const char *actual, const char *expected, const char *file, int line) +bool QTest::compare_helper(bool success, const char *failureMsg, + char *val1, char *val2, + const char *actual, const char *expected, + const char *file, int line) { - return QTestResult::compare(success, msg, val1, val2, actual, expected, file, line); + return QTestResult::compare(success, failureMsg, val1, val2, actual, expected, file, line); } /*! \fn bool QTest::qCompare(float const &t1, float const &t2, const char *actual, const char *expected, const char *file, int line) @@ -2430,10 +2449,8 @@ template <> Q_TESTLIB_EXPORT bool QTest::qCompare(float const &t1, float const &t2, const char *actual, const char *expected, const char *file, int line) { - return qFuzzyCompare(t1, t2) - ? compare_helper(true, "COMPARE()", file, line) - : compare_helper(false, "Compared floats are not the same (fuzzy compare)", - toString(t1), toString(t2), actual, expected, file, line); + return compare_helper(qFuzzyCompare(t1, t2), "Compared floats are not the same (fuzzy compare)", + toString(t1), toString(t2), actual, expected, file, line); } /*! \fn bool QTest::qCompare(double const &t1, double const &t2, const char *actual, const char *expected, const char *file, int line) @@ -2443,10 +2460,8 @@ template <> Q_TESTLIB_EXPORT bool QTest::qCompare(double const &t1, double const &t2, const char *actual, const char *expected, const char *file, int line) { - return qFuzzyCompare(t1, t2) - ? compare_helper(true, "COMPARE()", file, line) - : compare_helper(false, "Compared doubles are not the same (fuzzy compare)", - toString(t1), toString(t2), actual, expected, file, line); + return compare_helper(qFuzzyCompare(t1, t2), "Compared doubles are not the same (fuzzy compare)", + toString(t1), toString(t2), actual, expected, file, line); } #define TO_STRING_IMPL(TYPE, FORMAT) \ @@ -2499,10 +2514,8 @@ char *QTest::toString(const void *p) bool QTest::compare_string_helper(const char *t1, const char *t2, const char *actual, const char *expected, const char *file, int line) { - return (qstrcmp(t1, t2) == 0) - ? compare_helper(true, "COMPARE()", file, line) - : compare_helper(false, "Compared strings are not the same", - toString(t1), toString(t2), actual, expected, file, line); + return compare_helper(qstrcmp(t1, t2) == 0, "Compared strings are not the same", + toString(t1), toString(t2), actual, expected, file, line); } /*! \fn bool QTest::compare_ptr_helper(const void *t1, const void *t2, const char *actual, const char *expected, const char *file, int line); diff --git a/src/testlib/qtestcase.h b/src/testlib/qtestcase.h index c64420b0003..a344736f7b4 100644 --- a/src/testlib/qtestcase.h +++ b/src/testlib/qtestcase.h @@ -200,7 +200,8 @@ namespace QTest Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *msg, const char *file, int line); - Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *msg, char *val1, char *val2, + Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *failureMsg, + char *val1, char *val2, const char *actual, const char *expected, const char *file, int line); Q_TESTLIB_EXPORT void qSleep(int ms); @@ -217,13 +218,10 @@ namespace QTest inline bool qCompare(T const &t1, T const &t2, const char *actual, const char *expected, const char *file, int line) { - return (t1 == t2) - ? compare_helper(true, "COMPARE()", file, line) - : compare_helper(false, "Compared values are not the same", - toString(t1), toString(t2), actual, expected, file, line); + return compare_helper(t1 == t2, "Compared values are not the same", + toString(t1), toString(t2), actual, expected, file, line); } - template <> Q_TESTLIB_EXPORT bool qCompare(float const &t1, float const &t2, const char *actual, const char *expected, const char *file, int line); @@ -235,10 +233,8 @@ namespace QTest inline bool compare_ptr_helper(const void *t1, const void *t2, const char *actual, const char *expected, const char *file, int line) { - return (t1 == t2) - ? compare_helper(true, "COMPARE()", file, line) - : compare_helper(false, "Compared pointers are not the same", - toString(t1), toString(t2), actual, expected, file, line); + return compare_helper(t1 == t2, "Compared pointers are not the same", + toString(t1), toString(t2), actual, expected, file, line); } Q_TESTLIB_EXPORT bool compare_string_helper(const char *t1, const char *t2, const char *actual, diff --git a/src/testlib/qtestresult.cpp b/src/testlib/qtestresult.cpp index 14ab29af2b4..231fc8f3b1c 100644 --- a/src/testlib/qtestresult.cpp +++ b/src/testlib/qtestresult.cpp @@ -254,22 +254,38 @@ bool QTestResult::compare(bool success, const char *msg, const char *file, int l return checkStatement(success, msg, file, line); } -bool QTestResult::compare(bool success, const char *msg, char *val1, char *val2, - const char *actual, const char *expected, const char *file, int line) +bool QTestResult::compare(bool success, const char *failureMsg, + char *val1, char *val2, + const char *actual, const char *expected, + const char *file, int line) { QTEST_ASSERT(expected); QTEST_ASSERT(actual); - if (!val1 && !val2) - return compare(success, msg, file, line); + char msg[1024]; + + if (QTestLog::verboseLevel() >= 2) { + qsnprintf(msg, 1024, "QCOMPARE(%s, %s)", actual, expected); + QTestLog::info(msg, file, line); + } + + if (!failureMsg) + failureMsg = "Compared values are not the same"; + + if (success && QTest::expectFailMode) { + qsnprintf(msg, 1024, "QCOMPARE(%s, %s) returned TRUE unexpectedly.", actual, expected); + } else if (val1 || val2) { + qsnprintf(msg, 1024, "%s\n Actual (%s): %s\n Expected (%s): %s", + failureMsg, + actual, val1 ? val1 : "", + expected, val2 ? val2 : ""); + } else + qsnprintf(msg, 1024, "%s", failureMsg); - char buf[1024]; - qsnprintf(buf, 1024, "%s\n Actual (%s): %s\n Expected (%s): %s", msg, - actual, val1 ? val1 : "", - expected, val2 ? val2 : ""); delete [] val1; delete [] val2; - return compare(success, buf, file, line); + + return checkStatement(success, msg, file, line); } void QTestResult::addFailure(const char *message, const char *file, int line) diff --git a/src/testlib/qtestresult_p.h b/src/testlib/qtestresult_p.h index b060926f2ae..1bf070f6cf0 100644 --- a/src/testlib/qtestresult_p.h +++ b/src/testlib/qtestresult_p.h @@ -77,8 +77,10 @@ public: static void addFailure(const char *message, const char *file, int line); static bool compare(bool success, const char *msg, const char *file, int line); - static bool compare(bool success, const char *msg, char *val1, char *val2, - const char *actual, const char *expected, const char *file, int line); + static bool compare(bool success, const char *failureMsg, + char *val1, char *val2, + const char *actual, const char *expected, + const char *file, int line); static void setCurrentGlobalTestData(QTestData *data); static void setCurrentTestData(QTestData *data); diff --git a/tests/auto/testlib/selftests/expected_expectfail.lightxml b/tests/auto/testlib/selftests/expected_expectfail.lightxml index 34f4f1e70b3..55bd9578a87 100644 --- a/tests/auto/testlib/selftests/expected_expectfail.lightxml +++ b/tests/auto/testlib/selftests/expected_expectfail.lightxml @@ -132,7 +132,7 @@ - + diff --git a/tests/auto/testlib/selftests/expected_expectfail.txt b/tests/auto/testlib/selftests/expected_expectfail.txt index bd4e2172d65..6028b5ddfed 100644 --- a/tests/auto/testlib/selftests/expected_expectfail.txt +++ b/tests/auto/testlib/selftests/expected_expectfail.txt @@ -47,7 +47,7 @@ XPASS : tst_ExpectFail::xpass() 'true' returned TRUE unexpectedly. () XPASS : tst_ExpectFail::xpassDataDrivenWithQVerify(XPass) 'true' returned TRUE unexpectedly. () Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/expectfail/tst_expectfail.cpp(250)] PASS : tst_ExpectFail::xpassDataDrivenWithQVerify(Pass) -XPASS : tst_ExpectFail::xpassDataDrivenWithQCompare(XPass) COMPARE() +XPASS : tst_ExpectFail::xpassDataDrivenWithQCompare(XPass) QCOMPARE(1, 1) returned TRUE unexpectedly. Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/expectfail/tst_expectfail.cpp(271)] PASS : tst_ExpectFail::xpassDataDrivenWithQCompare(Pass) PASS : tst_ExpectFail::cleanupTestCase() diff --git a/tests/auto/testlib/selftests/expected_expectfail.xml b/tests/auto/testlib/selftests/expected_expectfail.xml index ff870a6a68f..e3200b00d64 100644 --- a/tests/auto/testlib/selftests/expected_expectfail.xml +++ b/tests/auto/testlib/selftests/expected_expectfail.xml @@ -134,7 +134,7 @@ - + diff --git a/tests/auto/testlib/selftests/expected_expectfail.xunitxml b/tests/auto/testlib/selftests/expected_expectfail.xunitxml index 6040af41ffa..45c260c77f0 100644 --- a/tests/auto/testlib/selftests/expected_expectfail.xunitxml +++ b/tests/auto/testlib/selftests/expected_expectfail.xunitxml @@ -45,7 +45,7 @@ - + diff --git a/tests/auto/testlib/selftests/expected_verbose2.lightxml b/tests/auto/testlib/selftests/expected_verbose2.lightxml index 1310f2bb09d..2937adbe014 100644 --- a/tests/auto/testlib/selftests/expected_verbose2.lightxml +++ b/tests/auto/testlib/selftests/expected_verbose2.lightxml @@ -12,7 +12,7 @@ - + @@ -23,7 +23,7 @@ - + @@ -36,7 +36,7 @@ - + @@ -53,7 +53,7 @@ - + @@ -78,7 +78,7 @@ - + @@ -123,7 +123,7 @@ - + diff --git a/tests/auto/testlib/selftests/expected_verbose2.txt b/tests/auto/testlib/selftests/expected_verbose2.txt index 9012a7c5692..34957f4ae76 100644 --- a/tests/auto/testlib/selftests/expected_verbose2.txt +++ b/tests/auto/testlib/selftests/expected_verbose2.txt @@ -5,18 +5,18 @@ PASS : tst_Counting::initTestCase() INFO : tst_Counting::testPassPass() entering INFO : tst_Counting::testPassPass(row 1) QVERIFY(true) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(111)] -INFO : tst_Counting::testPassPass(row 1) COMPARE() +INFO : tst_Counting::testPassPass(row 1) QCOMPARE(2 + 1, 3) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(112)] PASS : tst_Counting::testPassPass(row 1) INFO : tst_Counting::testPassPass(row 2) QVERIFY(true) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(111)] -INFO : tst_Counting::testPassPass(row 2) COMPARE() +INFO : tst_Counting::testPassPass(row 2) QCOMPARE(2 + 1, 3) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(112)] PASS : tst_Counting::testPassPass(row 2) INFO : tst_Counting::testPassSkip() entering INFO : tst_Counting::testPassSkip(row 1) QVERIFY(true) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(111)] -INFO : tst_Counting::testPassSkip(row 1) COMPARE() +INFO : tst_Counting::testPassSkip(row 1) QCOMPARE(2 + 1, 3) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(112)] PASS : tst_Counting::testPassSkip(row 1) SKIP : tst_Counting::testPassSkip(row 2) Skipping @@ -24,7 +24,7 @@ SKIP : tst_Counting::testPassSkip(row 2) Skipping INFO : tst_Counting::testPassFail() entering INFO : tst_Counting::testPassFail(row 1) QVERIFY(true) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(111)] -INFO : tst_Counting::testPassFail(row 1) COMPARE() +INFO : tst_Counting::testPassFail(row 1) QCOMPARE(2 + 1, 3) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(112)] PASS : tst_Counting::testPassFail(row 1) INFO : tst_Counting::testPassFail(row 2) QVERIFY(false) @@ -36,7 +36,7 @@ SKIP : tst_Counting::testSkipPass(row 1) Skipping Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(118)] INFO : tst_Counting::testSkipPass(row 2) QVERIFY(true) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(111)] -INFO : tst_Counting::testSkipPass(row 2) COMPARE() +INFO : tst_Counting::testSkipPass(row 2) QCOMPARE(2 + 1, 3) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(112)] PASS : tst_Counting::testSkipPass(row 2) INFO : tst_Counting::testSkipSkip() entering @@ -58,7 +58,7 @@ FAIL! : tst_Counting::testFailPass(row 1) 'false' returned FALSE. () Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(115)] INFO : tst_Counting::testFailPass(row 2) QVERIFY(true) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(111)] -INFO : tst_Counting::testFailPass(row 2) COMPARE() +INFO : tst_Counting::testFailPass(row 2) QCOMPARE(2 + 1, 3) Loc: [/home/user/dev/qt5/qtbase/tests/auto/testlib/selftests/counting/tst_counting.cpp(112)] PASS : tst_Counting::testFailPass(row 2) INFO : tst_Counting::testFailSkip() entering diff --git a/tests/auto/testlib/selftests/expected_verbose2.xml b/tests/auto/testlib/selftests/expected_verbose2.xml index 693ef2b1876..d181c6d2158 100644 --- a/tests/auto/testlib/selftests/expected_verbose2.xml +++ b/tests/auto/testlib/selftests/expected_verbose2.xml @@ -14,7 +14,7 @@ - + @@ -25,7 +25,7 @@ - + @@ -38,7 +38,7 @@ - + @@ -55,7 +55,7 @@ - + @@ -80,7 +80,7 @@ - + @@ -125,7 +125,7 @@ - + diff --git a/tests/auto/testlib/selftests/expected_verbose2.xunitxml b/tests/auto/testlib/selftests/expected_verbose2.xunitxml index 8b9ed5257d2..a774cb9d9f5 100644 --- a/tests/auto/testlib/selftests/expected_verbose2.xunitxml +++ b/tests/auto/testlib/selftests/expected_verbose2.xunitxml @@ -7,25 +7,25 @@ - + - + - + - + - + @@ -40,7 +40,7 @@ - + @@ -70,25 +70,25 @@ - + - + - + - + - + - + diff --git a/tests/auto/testlib/selftests/tst_selftests.cpp b/tests/auto/testlib/selftests/tst_selftests.cpp index 5d216992d7c..a9ec4e31f2a 100644 --- a/tests/auto/testlib/selftests/tst_selftests.cpp +++ b/tests/auto/testlib/selftests/tst_selftests.cpp @@ -104,7 +104,7 @@ inline bool qCompare } if (qAbs(qreal(r1.total) - qreal(r2.total)) <= qreal(r1.total)*variance) { - return compare_helper(true, "COMPARE()", file, line); + return compare_helper(true, 0, 0, 0, actual, expected, file, line); } // Whoops, didn't match. Compare the whole string for the most useful failure message.