Fix argv hacking in tst_benchlibcallgrind.cpp

It unconditionally added -callgrind to its own command-line options,
but the way testlib handles this argument is, in QTest::qRun(), to
re-run the program under the control of valgrind --tool=callgrind,
removing the -callgrind command-line option from the test and adding
-callgrindchild to its command-line options. So we shouldn't re-add
the -callgrind option in the resulting recursive call.

The test now runs quickly, producing sensible output, where previously
it took a very long time. Revised the drivers to reflect this
speed-up, but continue skipping the non-.txt formats to save the need
for variant-output files for many formats. To match that, removed the
unused non-.txt results files.

Change-Id: Iaa99c1b5964d50bccfc6076a21896791b6bbf289
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

9 files changed, 23 insertions, 85 deletions

diff --git a/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp b/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp
index 51fb4d358a..353a7f5018 100644
--- a/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp
+++ b/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp
@@ -26,9 +26,8 @@
 **
 ****************************************************************************/
 
-
-#include <QtCore/QCoreApplication>
 #include <QTest>
+#include <QtCore/QCoreApplication>
 
 #if __has_include(<valgrind/valgrind.h>)
 #  include <valgrind/valgrind.h>
@@ -82,9 +81,17 @@ void tst_BenchlibCallgrind::twoHundredMillionInstructions()
 int main(int argc, char *argv[])
 {
     std::vector<const char*> args(argv, argv + argc);
-    args.push_back("-callgrind");
-    argc = args.size();
-    argv = const_cast<char**>(&args[0]);
+    // Add the -callgrind argument unless (it's there anyway or) we're the
+    // recursive invocation with -callgrindchild passed.
+    if (std::find_if(args.begin(), args.end(),
+                     [](const char *arg) {
+                         return qstrcmp(arg, "-callgrindchild") == 0
+                             || qstrcmp(arg, "-callgrind") == 0;
+                     }) == args.end()) {
+        args.push_back("-callgrind");
+        argc = args.size();
+        argv = const_cast<char**>(&args[0]);
+    }
 
     QTEST_MAIN_IMPL(tst_BenchlibCallgrind)
 }
diff --git a/tests/auto/testlib/selftests/expected_benchlibcallgrind.csv b/tests/auto/testlib/selftests/expected_benchlibcallgrind.csv
deleted file mode 100644
index 6ce2e2ced8..0000000000
--- a/tests/auto/testlib/selftests/expected_benchlibcallgrind.csv
+++ /dev/null
@@ -1 +0,0 @@
-"twoHundredMillionInstructions","","InstructionReads",200000158,200000158,1
diff --git a/tests/auto/testlib/selftests/expected_benchlibcallgrind.junitxml b/tests/auto/testlib/selftests/expected_benchlibcallgrind.junitxml
deleted file mode 100644
index 0850e74733..0000000000
--- a/tests/auto/testlib/selftests/expected_benchlibcallgrind.junitxml
+++ /dev/null
@@ -1,13 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" ?>
-<testsuite name="tst_BenchlibCallgrind" timestamp="@TEST_START_TIME@" hostname="@HOSTNAME@" tests="3" failures="0" errors="0" skipped="1" time="@TEST_DURATION@">
-  <properties>
-    <property name="QTestVersion" value="@INSERT_QT_VERSION_HERE@"/>
-    <property name="QtVersion" value="@INSERT_QT_VERSION_HERE@"/>
-    <property name="QtBuild" value=""/>
-  </properties>
-  <testcase name="initTestCase" classname="tst_BenchlibCallgrind" time="@TEST_DURATION@"/>
-  <testcase name="twoHundredMillionInstructions" classname="tst_BenchlibCallgrind" time="@TEST_DURATION@">
-    <skipped message="This test is only defined for gcc and x86."/>
-  </testcase>
-  <testcase name="cleanupTestCase" classname="tst_BenchlibCallgrind" time="@TEST_DURATION@"/>
-</testsuite>
diff --git a/tests/auto/testlib/selftests/expected_benchlibcallgrind.lightxml b/tests/auto/testlib/selftests/expected_benchlibcallgrind.lightxml
deleted file mode 100644
index dab06896d0..0000000000
--- a/tests/auto/testlib/selftests/expected_benchlibcallgrind.lightxml
+++ /dev/null
@@ -1,20 +0,0 @@
-<Environment>
-    <QtVersion>@INSERT_QT_VERSION_HERE@</QtVersion>
-    <QtBuild/>
-    <QTestVersion>@INSERT_QT_VERSION_HERE@</QTestVersion>
-</Environment>
-<TestFunction name="initTestCase">
-<Incident type="pass" file="" line="0" />
-    <Duration msecs="0"/>
-</TestFunction>
-<TestFunction name="twoHundredMillionInstructions">
-<Message type="skip" file="qtbase/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp" line="0">
-    <Description><![CDATA[This test is only defined for gcc and x86.]]></Description>
-</Message>
-    <Duration msecs="0"/>
-</TestFunction>
-<TestFunction name="cleanupTestCase">
-<Incident type="pass" file="" line="0" />
-    <Duration msecs="0"/>
-</TestFunction>
-<Duration msecs="0"/>
diff --git a/tests/auto/testlib/selftests/expected_benchlibcallgrind.tap b/tests/auto/testlib/selftests/expected_benchlibcallgrind.tap
deleted file mode 100644
index da30082eaf..0000000000
--- a/tests/auto/testlib/selftests/expected_benchlibcallgrind.tap
+++ /dev/null
@@ -1,9 +0,0 @@
-TAP version 13
-# tst_BenchlibCallgrind
-ok 1 - initTestCase()
-ok 2 - twoHundredMillionInstructions() # SKIP This test is only defined for gcc and x86.
-ok 3 - cleanupTestCase()
-1..3
-# tests 3
-# pass 2
-# fail 0
diff --git a/tests/auto/testlib/selftests/expected_benchlibcallgrind.teamcity b/tests/auto/testlib/selftests/expected_benchlibcallgrind.teamcity
deleted file mode 100644
index 9499fed8ce..0000000000
--- a/tests/auto/testlib/selftests/expected_benchlibcallgrind.teamcity
+++ /dev/null
@@ -1,7 +0,0 @@
-##teamcity[testSuiteStarted name='tst_BenchlibCallgrind' flowId='tst_BenchlibCallgrind']
-##teamcity[testStarted name='initTestCase()' flowId='tst_BenchlibCallgrind']
-##teamcity[testFinished name='initTestCase()' flowId='tst_BenchlibCallgrind']
-##teamcity[testIgnored name='twoHundredMillionInstructions()' message='This test is only defined for gcc and x86. |[Loc: qtbase/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp(0)|]' flowId='tst_BenchlibCallgrind']
-##teamcity[testStarted name='cleanupTestCase()' flowId='tst_BenchlibCallgrind']
-##teamcity[testFinished name='cleanupTestCase()' flowId='tst_BenchlibCallgrind']
-##teamcity[testSuiteFinished name='tst_BenchlibCallgrind' flowId='tst_BenchlibCallgrind']
diff --git a/tests/auto/testlib/selftests/expected_benchlibcallgrind.xml b/tests/auto/testlib/selftests/expected_benchlibcallgrind.xml
deleted file mode 100644
index 14ab86b45d..0000000000
--- a/tests/auto/testlib/selftests/expected_benchlibcallgrind.xml
+++ /dev/null
@@ -1,23 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<TestCase name="tst_BenchlibCallgrind">
-<Environment>
-    <QtVersion>@INSERT_QT_VERSION_HERE@</QtVersion>
-    <QtBuild/>
-    <QTestVersion>@INSERT_QT_VERSION_HERE@</QTestVersion>
-</Environment>
-<TestFunction name="initTestCase">
-<Incident type="pass" file="" line="0" />
-    <Duration msecs="0"/>
-</TestFunction>
-<TestFunction name="twoHundredMillionInstructions">
-<Message type="skip" file="qtbase/tests/auto/testlib/selftests/benchlibcallgrind/tst_benchlibcallgrind.cpp" line="0">
-    <Description><![CDATA[This test is only defined for gcc and x86.]]></Description>
-</Message>
-    <Duration msecs="0"/>
-</TestFunction>
-<TestFunction name="cleanupTestCase">
-<Incident type="pass" file="" line="0" />
-    <Duration msecs="0"/>
-</TestFunction>
-<Duration msecs="0"/>
-</TestCase>
diff --git a/tests/auto/testlib/selftests/generate_expected_output.py b/tests/auto/testlib/selftests/generate_expected_output.py
index fb62e3eaf3..677b65b33b 100755
--- a/tests/auto/testlib/selftests/generate_expected_output.py
+++ b/tests/auto/testlib/selftests/generate_expected_output.py
@@ -325,7 +325,7 @@ def main(argv):
     argument_parser.add_argument('--formats', '-f',
                                  help='Comma-separated list of formats')
     argument_parser.add_argument('--skip-callgrind', '-s', action='store_true',
-                                 help='Skip the expensive benchlib callgrind test')
+                                 help='Skip the (no longer expensive) benchlib callgrind test')
     argument_parser.add_argument('subtests', help='subtests to regenerate',
                                  nargs='*', type=str)
 
diff --git a/tests/auto/testlib/selftests/tst_selftests.cpp b/tests/auto/testlib/selftests/tst_selftests.cpp
index c5ae337629..ca8fee582a 100644
--- a/tests/auto/testlib/selftests/tst_selftests.cpp
+++ b/tests/auto/testlib/selftests/tst_selftests.cpp
@@ -719,8 +719,11 @@ bool TestLogger::shouldIgnoreTest(const QString &test) const
             || test == "silent")
             return true;
 
-        // `crashes' will not output valid XML on platforms without a crash handler
-        if (test == "crashes")
+        // These tests produce variable output (callgrind because of #if-ery,
+        // crashes by virtue of platform differences in where the output cuts
+        // off), so only test them for one format, to avoid the need for several
+        // _n variants for each format. Also, crashes can produce invalid XML.
+        if (test == "crashes" || test == "benchlibcallgrind")
             return true;
 
         // this test prints out some floats in the testlog and the formatting is
@@ -728,10 +731,11 @@ bool TestLogger::shouldIgnoreTest(const QString &test) const
         if (test == "float")
             return true;
 
-        // these tests are quite slow, and running them for all the loggers significantly
-        // increases the overall test time.  They do not really relate to logging, so it
-        // should be safe to run them just for the stdout loggers.
-        if (test == "benchlibcallgrind" || test == "sleep")
+        // This test is quite slow, and running it for all the loggers
+        // significantly increases the overall test time.  It does not really
+        // relate to logging, so it should be safe to run it just for the stdout
+        // loggers.
+        if (test == "sleep")
             return true;
     }