From e42fe5c775f982e89772eab5083b6dc641a870e3 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Tue, 30 Nov 2021 09:54:03 +0100 Subject: Fix argv hacking in tst_benchlibcallgrind.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../benchlibcallgrind/tst_benchlibcallgrind.cpp | 17 +++++++++++----- .../selftests/expected_benchlibcallgrind.csv | 1 - .../selftests/expected_benchlibcallgrind.junitxml | 13 ------------ .../selftests/expected_benchlibcallgrind.lightxml | 20 ------------------- .../selftests/expected_benchlibcallgrind.tap | 9 --------- .../selftests/expected_benchlibcallgrind.teamcity | 7 ------- .../selftests/expected_benchlibcallgrind.xml | 23 ---------------------- .../testlib/selftests/generate_expected_output.py | 2 +- tests/auto/testlib/selftests/tst_selftests.cpp | 16 +++++++++------ 9 files changed, 23 insertions(+), 85 deletions(-) delete mode 100644 tests/auto/testlib/selftests/expected_benchlibcallgrind.csv delete mode 100644 tests/auto/testlib/selftests/expected_benchlibcallgrind.junitxml delete mode 100644 tests/auto/testlib/selftests/expected_benchlibcallgrind.lightxml delete mode 100644 tests/auto/testlib/selftests/expected_benchlibcallgrind.tap delete mode 100644 tests/auto/testlib/selftests/expected_benchlibcallgrind.teamcity delete mode 100644 tests/auto/testlib/selftests/expected_benchlibcallgrind.xml (limited to 'tests') 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 #include +#include #if __has_include() # include @@ -82,9 +81,17 @@ void tst_BenchlibCallgrind::twoHundredMillionInstructions() int main(int argc, char *argv[]) { std::vector args(argv, argv + argc); - args.push_back("-callgrind"); - argc = args.size(); - argv = const_cast(&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(&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 @@ - - - - - - - - - - - - - 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 @@ - - @INSERT_QT_VERSION_HERE@ - - @INSERT_QT_VERSION_HERE@ - - - - - - - - - - - - - - - - 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 @@ - - - - @INSERT_QT_VERSION_HERE@ - - @INSERT_QT_VERSION_HERE@ - - - - - - - - - - - - - - - - - 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; } -- cgit v1.2.3