Skip to content

Commit

Permalink
Revert "QTest: add -[no]throwon{fail,skip} command line arguments"
Browse files Browse the repository at this point in the history
This reverts commit fde5730 and a
portion of commit 627f804 and of commit
e769cf0.

The command-line feature and the environment variables are a design
flaw, because there are NO conditions under which a regular user who
isn't the designer of the test can make the correct choice. Either the
tests are designed to work with throwing or they are not. The test must
be run in the mode that the test developer wrote it.

[ChangeLog][QtTest] The command-line options and environment variables
that changed the defaults for QTest::setThrowOn{Fail,Skip}() have been
removed due to a design flaw. To set the default, either place a call to
the required function in the test's initTestCase() function or define
the C++ macro.

Task-number: QTBUG-66320
Change-Id: I0c17899984ebfd70de97fffd51784aa42d41b743
Reviewed-by: Jason McDonald <macadder1@gmail.com>
  • Loading branch information
thiagomacieira committed Nov 8, 2024
1 parent 519d3d3 commit ad568b8
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 51 deletions.
6 changes: 0 additions & 6 deletions src/testlib/doc/src/qttestlib-manual.qdoc
Original file line number Diff line number Diff line change
Expand Up @@ -415,12 +415,6 @@
to e.g. debug an unstable or intermittent failure in a test, by
launching the test in a debugger. Support for this variable was
added in Qt 6.1.
\li \c QTEST_THROW_ON_FAIL (since 6.8) \br
Setting this variable to a non-zero value will cause QCOMPARE()/QVERIFY()
etc to throw on failure (as opposed to just returning from the
immediately-surrounding function scope).
\li \c QTEST_THROW_ON_SKIP (since 6.8) \br
Same as \c QTEST_THROW_ON_FAIL, except affecting QSKIP().
\endlist

\section1 Creating a Benchmark
Expand Down
23 changes: 0 additions & 23 deletions src/testlib/qtestcase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,6 @@ void Internal::maybeThrowOnSkip()
with \c{true}, you need to call it \e{N} times with \c{false} to get back
to where you started.
The default is \c{false}, unless the \l{Qt Test Environment Variables}
{QTEST_THROW_ON_FAIL environment variable} is set.
This call has no effect when the \l{QTEST_THROW_ON_FAIL} C++ macro is
defined.
Expand All @@ -332,9 +329,6 @@ void setThrowOnFail(bool enable) noexcept
with \c{true}, you need to call it \e{N} times with \c{false} to get back
to where you started.
The default is \c{false}, unless the \l{Qt Test Environment Variables}
{QTEST_THROW_ON_SKIP environment variable} is set.
This call has no effect when the \l{QTEST_THROW_ON_SKIP} C++ macro is
defined.
Expand Down Expand Up @@ -616,11 +610,6 @@ Q_TESTLIB_EXPORT void qtest_qParseArgs(int argc, const char *const argv[], bool
QTest::testFunctions.clear();
QTest::testTags.clear();

if (qEnvironmentVariableIsSet("QTEST_THROW_ON_FAIL"))
QTest::setThrowOnFail(true);
if (qEnvironmentVariableIsSet("QTEST_THROW_ON_SKIP"))
QTest::setThrowOnSkip(true);

#if defined(Q_OS_DARWIN) && defined(HAVE_XCTEST)
if (QXcodeTestLogger::canLogTestProgress())
logFormat = QTestLog::XCTest;
Expand Down Expand Up @@ -677,10 +666,6 @@ Q_TESTLIB_EXPORT void qtest_qParseArgs(int argc, const char *const argv[], bool
" repeated forever. This is intended as a developer tool, and\n"
" is only supported with the plain text logger.\n"
" -skipblacklisted : Skip blacklisted tests. Useful for measuring test coverage.\n"
" -[no]throwonfail : Enables/disables throwing on QCOMPARE()/QVERIFY()/etc.\n"
" Default: off, unless QTEST_THROW_ON_FAIL is set.\n"
" -[no]throwonskip : Enables/disables throwing on QSKIP().\n"
" Default: off, unless QTEST_THROW_ON_SKIP is set.\n"
"\n"
" Benchmarking options:\n"
#if QT_CONFIG(valgrind)
Expand Down Expand Up @@ -841,14 +826,6 @@ Q_TESTLIB_EXPORT void qtest_qParseArgs(int argc, const char *const argv[], bool
QTest::Internal::noCrashHandler = true;
} else if (strcmp(argv[i], "-skipblacklisted") == 0) {
QTest::skipBlacklisted = true;
} else if (strcmp(argv[i], "-throwonfail") == 0) {
QTest::setThrowOnFail(true);
} else if (strcmp(argv[i], "-nothrowonfail") == 0) {
QTest::setThrowOnFail(false);
} else if (strcmp(argv[i], "-throwonskip") == 0) {
QTest::setThrowOnSkip(true);
} else if (strcmp(argv[i], "-nothrowonskip") == 0) {
QTest::setThrowOnSkip(false);
#if QT_CONFIG(valgrind)
} else if (strcmp(argv[i], "-callgrind") == 0) {
if (!QBenchmarkValgrindUtils::haveValgrind()) {
Expand Down
29 changes: 7 additions & 22 deletions tests/auto/testlib/selftests/tst_selftests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ struct TestLogger
return outputFile.readAll();
}

bool shouldIgnoreTest(const QString &test, Throw throwing) const;
bool shouldIgnoreTest(const QString &test) const;

operator QTestLog::LogMode() const { return logger; }

Expand All @@ -633,19 +633,13 @@ struct TestLogger
ArgumentStyle argumentStyle = NewStyleArgument;
};

bool TestLogger::shouldIgnoreTest(const QString &test, Throw throwing) const
bool TestLogger::shouldIgnoreTest(const QString &test) const
{
#if defined(QT_USE_APPLE_UNIFIED_LOGGING)
if (logger == QTestLog::Apple)
return true;
#endif

if (throwing == Throw::OnFail && test == "cmptest") {
// This test requires continuing the same test function after failure,
// so the output is different with this flag.
return true;
}

// These tests are affected by timing and whether the CPU tick counter
// is monotonically increasing. They won't work on some machines so
// leave them off by default. Feel free to enable them for your own
Expand Down Expand Up @@ -1031,11 +1025,11 @@ TestProcessResult runTestProcess(const QString &test, const QStringList &argumen
/*
Runs a single test and verifies the output against the expected results.
*/
void runTest(const QString &test, const TestLoggers &requestedLoggers, Throw throwing = {})
void runTest(const QString &test, const TestLoggers &requestedLoggers)
{
TestLoggers loggers;
for (auto logger : requestedLoggers) {
if (!logger.shouldIgnoreTest(test, throwing))
if (!logger.shouldIgnoreTest(test))
loggers += logger;
}

Expand All @@ -1045,10 +1039,6 @@ void runTest(const QString &test, const TestLoggers &requestedLoggers, Throw thr
QStringList arguments;
for (auto logger : loggers)
arguments += logger.arguments(test);
if (throwing == Throw::OnFail) // don't distinguish between throwonfail/throwonskip
arguments += {"-throwonfail", "-throwonskip"};
else
arguments += {"-nothrowonfail", "-nothrowonskip"};

CAPTURE(test);
CAPTURE(arguments);
Expand All @@ -1075,9 +1065,9 @@ void runTest(const QString &test, const TestLoggers &requestedLoggers, Throw thr
/*
Runs a single test and verifies the output against the expected result.
*/
void runTest(const QString &test, const TestLogger &logger, Throw t = {})
void runTest(const QString &test, const TestLogger &logger)
{
runTest(test, TestLoggers{logger}, t);
runTest(test, TestLoggers{logger});
}

// ----------------------- Catch helpers -----------------------
Expand Down Expand Up @@ -1220,12 +1210,7 @@ SCENARIO("Test output of the loggers is as expected")
GIVEN("The " << logger << " logger") {
for (QString test : tests) {
AND_GIVEN("The " << test << " subtest") {
WHEN("Throwing on failure or skip") {
runTest(test, TestLogger(logger, StdoutOutput), Throw::OnFail);
}
WHEN("Returning on failure or skip") {
runTest(test, TestLogger(logger, StdoutOutput));
}
runTest(test, TestLogger(logger, StdoutOutput));
}
}
}
Expand Down

0 comments on commit ad568b8

Please sign in to comment.