From 2c06ee94dccbea06a7da22e52c3e41819103a262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sun, 16 Jun 2019 16:12:47 +0200 Subject: [PATCH] Make --list-* exit code be 0 Previously it returned the sum of listed things because ???. This was completely useless and in many ways actively counterproductive because of the success/failure conventions around exit codes. Closes #1410 --- docs/release-notes.md | 2 + include/internal/catch_list.cpp | 231 ++++++++++++++++------------- include/internal/catch_list.h | 21 +-- include/internal/catch_session.cpp | 5 +- projects/CMakeLists.txt | 27 ++-- 5 files changed, 148 insertions(+), 138 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 46e48047d4..0595961759 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -37,6 +37,8 @@ ### Fixes * The `INFO` macro no longer contains superfluous semicolon (#1456) +* The `--list*` family of command line flags now return 0 on success (#1410, #1146) + ## 2.10.2 diff --git a/include/internal/catch_list.cpp b/include/internal/catch_list.cpp index 11e5514044..f8b3503f7d 100644 --- a/include/internal/catch_list.cpp +++ b/include/internal/catch_list.cpp @@ -24,59 +24,125 @@ #include #include #include +#include namespace Catch { + namespace { - std::size_t listTests( Config const& config ) { - TestSpec testSpec = config.testSpec(); - if( config.hasTestFilters() ) - Catch::cout() << "Matching test cases:\n"; - else { - Catch::cout() << "All available test cases:\n"; + struct TagInfo { + void add(std::string const& spelling); + std::string all() const; + + std::set spellings; + std::size_t count = 0; + }; + + + void listTests(Config const& config) { + TestSpec testSpec = config.testSpec(); + if (config.hasTestFilters()) + Catch::cout() << "Matching test cases:\n"; + else { + Catch::cout() << "All available test cases:\n"; + } + + auto matchedTestCases = filterTests(getAllTestCasesSorted(config), testSpec, config); + for (auto const& testCaseInfo : matchedTestCases) { + Colour::Code colour = testCaseInfo.isHidden() + ? Colour::SecondaryText + : Colour::None; + Colour colourGuard(colour); + + Catch::cout() << Column(testCaseInfo.name).initialIndent(2).indent(4) << "\n"; + if (config.verbosity() >= Verbosity::High) { + Catch::cout() << Column(Catch::Detail::stringify(testCaseInfo.lineInfo)).indent(4) << std::endl; + std::string description = testCaseInfo.description; + if (description.empty()) + description = "(NO DESCRIPTION)"; + Catch::cout() << Column(description).indent(4) << std::endl; + } + if (!testCaseInfo.tags.empty()) + Catch::cout() << Column(testCaseInfo.tagsAsString()).indent(6) << "\n"; + } + + if (!config.hasTestFilters()) + Catch::cout() << pluralise(matchedTestCases.size(), "test case") << '\n' << std::endl; + else + Catch::cout() << pluralise(matchedTestCases.size(), "matching test case") << '\n' << std::endl; } - auto matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config ); - for( auto const& testCaseInfo : matchedTestCases ) { - Colour::Code colour = testCaseInfo.isHidden() - ? Colour::SecondaryText - : Colour::None; - Colour colourGuard( colour ); - - Catch::cout() << Column( testCaseInfo.name ).initialIndent( 2 ).indent( 4 ) << "\n"; - if( config.verbosity() >= Verbosity::High ) { - Catch::cout() << Column( Catch::Detail::stringify( testCaseInfo.lineInfo ) ).indent(4) << std::endl; - std::string description = testCaseInfo.description; - if( description.empty() ) - description = "(NO DESCRIPTION)"; - Catch::cout() << Column( description ).indent(4) << std::endl; + void listTestsNamesOnly(Config const& config) { + TestSpec testSpec = config.testSpec(); + std::size_t matchedTests = 0; + std::vector matchedTestCases = filterTests(getAllTestCasesSorted(config), testSpec, config); + for (auto const& testCaseInfo : matchedTestCases) { + matchedTests++; + if (startsWith(testCaseInfo.name, '#')) + Catch::cout() << '"' << testCaseInfo.name << '"'; + else + Catch::cout() << testCaseInfo.name; + if (config.verbosity() >= Verbosity::High) + Catch::cout() << "\t@" << testCaseInfo.lineInfo; + Catch::cout() << std::endl; } - if( !testCaseInfo.tags.empty() ) - Catch::cout() << Column( testCaseInfo.tagsAsString() ).indent( 6 ) << "\n"; } - if( !config.hasTestFilters() ) - Catch::cout() << pluralise( matchedTestCases.size(), "test case" ) << '\n' << std::endl; - else - Catch::cout() << pluralise( matchedTestCases.size(), "matching test case" ) << '\n' << std::endl; - return matchedTestCases.size(); - } + void listTags(Config const& config) { + TestSpec testSpec = config.testSpec(); + if (config.hasTestFilters()) + Catch::cout() << "Tags for matching test cases:\n"; + else { + Catch::cout() << "All available tags:\n"; + } - std::size_t listTestsNamesOnly( Config const& config ) { - TestSpec testSpec = config.testSpec(); - std::size_t matchedTests = 0; - std::vector matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config ); - for( auto const& testCaseInfo : matchedTestCases ) { - matchedTests++; - if( startsWith( testCaseInfo.name, '#' ) ) - Catch::cout() << '"' << testCaseInfo.name << '"'; - else - Catch::cout() << testCaseInfo.name; - if ( config.verbosity() >= Verbosity::High ) - Catch::cout() << "\t@" << testCaseInfo.lineInfo; + std::map tagCounts; + + std::vector matchedTestCases = filterTests(getAllTestCasesSorted(config), testSpec, config); + for (auto const& testCase : matchedTestCases) { + for (auto const& tagName : testCase.getTestCaseInfo().tags) { + std::string lcaseTagName = toLower(tagName); + auto countIt = tagCounts.find(lcaseTagName); + if (countIt == tagCounts.end()) + countIt = tagCounts.insert(std::make_pair(lcaseTagName, TagInfo())).first; + countIt->second.add(tagName); + } + } + + for (auto const& tagCount : tagCounts) { + ReusableStringStream rss; + rss << " " << std::setw(2) << tagCount.second.count << " "; + auto str = rss.str(); + auto wrapper = Column(tagCount.second.all()) + .initialIndent(0) + .indent(str.size()) + .width(CATCH_CONFIG_CONSOLE_WIDTH - 10); + Catch::cout() << str << wrapper << '\n'; + } + Catch::cout() << pluralise(tagCounts.size(), "tag") << '\n' << std::endl; + } + + void listReporters() { + Catch::cout() << "Available reporters:\n"; + IReporterRegistry::FactoryMap const& factories = getRegistryHub().getReporterRegistry().getFactories(); + std::size_t maxNameLen = 0; + for (auto const& factoryKvp : factories) + maxNameLen = (std::max)(maxNameLen, factoryKvp.first.size()); + + for (auto const& factoryKvp : factories) { + Catch::cout() + << Column(factoryKvp.first + ":") + .indent(2) + .width(5 + maxNameLen) + + Column(factoryKvp.second->getDescription()) + .initialIndent(0) + .indent(2) + .width(CATCH_CONFIG_CONSOLE_WIDTH - maxNameLen - 8) + << "\n"; + } Catch::cout() << std::endl; } - return matchedTests; - } + + } // end anonymous namespace void TagInfo::add( std::string const& spelling ) { ++count; @@ -99,75 +165,26 @@ namespace Catch { return out; } - std::size_t listTags( Config const& config ) { - TestSpec testSpec = config.testSpec(); - if( config.hasTestFilters() ) - Catch::cout() << "Tags for matching test cases:\n"; - else { - Catch::cout() << "All available tags:\n"; + bool list( std::shared_ptr const& config ) { + bool listed = false; + getCurrentMutableContext().setConfig( config ); + if (config->listTests()) { + listed = true; + listTests(*config); } - - std::map tagCounts; - - std::vector matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config ); - for( auto const& testCase : matchedTestCases ) { - for( auto const& tagName : testCase.getTestCaseInfo().tags ) { - std::string lcaseTagName = toLower( tagName ); - auto countIt = tagCounts.find( lcaseTagName ); - if( countIt == tagCounts.end() ) - countIt = tagCounts.insert( std::make_pair( lcaseTagName, TagInfo() ) ).first; - countIt->second.add( tagName ); - } + if (config->listTestNamesOnly()) { + listed = true; + listTestsNamesOnly(*config); } - - for( auto const& tagCount : tagCounts ) { - ReusableStringStream rss; - rss << " " << std::setw(2) << tagCount.second.count << " "; - auto str = rss.str(); - auto wrapper = Column( tagCount.second.all() ) - .initialIndent( 0 ) - .indent( str.size() ) - .width( CATCH_CONFIG_CONSOLE_WIDTH-10 ); - Catch::cout() << str << wrapper << '\n'; + if (config->listTags()) { + listed = true; + listTags(*config); } - Catch::cout() << pluralise( tagCounts.size(), "tag" ) << '\n' << std::endl; - return tagCounts.size(); - } - - std::size_t listReporters() { - Catch::cout() << "Available reporters:\n"; - IReporterRegistry::FactoryMap const& factories = getRegistryHub().getReporterRegistry().getFactories(); - std::size_t maxNameLen = 0; - for( auto const& factoryKvp : factories ) - maxNameLen = (std::max)( maxNameLen, factoryKvp.first.size() ); - - for( auto const& factoryKvp : factories ) { - Catch::cout() - << Column( factoryKvp.first + ":" ) - .indent(2) - .width( 5+maxNameLen ) - + Column( factoryKvp.second->getDescription() ) - .initialIndent(0) - .indent(2) - .width( CATCH_CONFIG_CONSOLE_WIDTH - maxNameLen-8 ) - << "\n"; + if (config->listReporters()) { + listed = true; + listReporters(); } - Catch::cout() << std::endl; - return factories.size(); - } - - Option list( std::shared_ptr const& config ) { - Option listedCount; - getCurrentMutableContext().setConfig( config ); - if( config->listTests() ) - listedCount = listedCount.valueOr(0) + listTests( *config ); - if( config->listTestNamesOnly() ) - listedCount = listedCount.valueOr(0) + listTestsNamesOnly( *config ); - if( config->listTags() ) - listedCount = listedCount.valueOr(0) + listTags( *config ); - if( config->listReporters() ) - listedCount = listedCount.valueOr(0) + listReporters(); - return listedCount; + return listed; } } // end namespace Catch diff --git a/include/internal/catch_list.h b/include/internal/catch_list.h index cea7bbabf2..70567b1199 100644 --- a/include/internal/catch_list.h +++ b/include/internal/catch_list.h @@ -8,30 +8,11 @@ #ifndef TWOBLUECUBES_CATCH_LIST_H_INCLUDED #define TWOBLUECUBES_CATCH_LIST_H_INCLUDED -#include "catch_option.hpp" #include "catch_config.hpp" -#include - namespace Catch { - std::size_t listTests( Config const& config ); - - std::size_t listTestsNamesOnly( Config const& config ); - - struct TagInfo { - void add( std::string const& spelling ); - std::string all() const; - - std::set spellings; - std::size_t count = 0; - }; - - std::size_t listTags( Config const& config ); - - std::size_t listReporters(); - - Option list( std::shared_ptr const& config ); + bool list( std::shared_ptr const& config ); } // end namespace Catch diff --git a/include/internal/catch_session.cpp b/include/internal/catch_session.cpp index b1d7a404b1..f490c88ab0 100644 --- a/include/internal/catch_session.cpp +++ b/include/internal/catch_session.cpp @@ -288,8 +288,9 @@ namespace Catch { applyFilenamesAsTags( *m_config ); // Handle list request - if( Option listed = list( m_config ) ) - return static_cast( *listed ); + if (list(m_config)) { + return 0; + } TestGroup tests { m_config }; auto const totals = tests.execute(); diff --git a/projects/CMakeLists.txt b/projects/CMakeLists.txt index 0aafc1780d..e0e7b63ed7 100644 --- a/projects/CMakeLists.txt +++ b/projects/CMakeLists.txt @@ -376,24 +376,33 @@ set_tests_properties(RunTests PROPERTIES FAIL_REGULAR_EXPRESSION "Filters:" ) -add_test(NAME ListTests COMMAND $ --list-tests --verbosity high) -set_tests_properties(ListTests PROPERTIES +# Because CTest does not allow us to check both return code _and_ expected +# output in one test, we run these commands twice. First time we check +# the output, the second time we check the exit code. +add_test(NAME List::Tests::Output COMMAND $ --list-tests --verbosity high) +set_tests_properties(List::Tests::Output PROPERTIES PASS_REGULAR_EXPRESSION "[0-9]+ test cases" FAIL_REGULAR_EXPRESSION "Hidden Test" ) +add_test(NAME List::Tests::ExitCode COMMAND $ --list-tests --verbosity high) -add_test(NAME ListTags COMMAND $ --list-tags) -set_tests_properties(ListTags PROPERTIES - PASS_REGULAR_EXPRESSION "[0-9]+ tags" +add_test(NAME List::Tags::Output COMMAND $ --list-tags) +set_tests_properties(List::Tags::Output PROPERTIES + PASS_REGULAR_EXPRESSION "[0-9]+ tags" FAIL_REGULAR_EXPRESSION "\\[\\.\\]") +add_test(NAME List::Tags::ExitCode COMMAND $ --list-tags) -add_test(NAME ListReporters COMMAND $ --list-reporters) -set_tests_properties(ListReporters PROPERTIES PASS_REGULAR_EXPRESSION "Available reporters:") +add_test(NAME List::Reporters::Output COMMAND $ --list-reporters) +set_tests_properties(List::Reporters::Output PROPERTIES PASS_REGULAR_EXPRESSION "Available reporters:") +add_test(NAME List::Reporters::ExitCode COMMAND $ --list-reporters) -add_test(NAME ListTestNamesOnly COMMAND $ --list-test-names-only) -set_tests_properties(ListTestNamesOnly PROPERTIES +add_test(NAME List::Tests::NamesOnly::Output COMMAND $ --list-test-names-only) +set_tests_properties(List::Tests::NamesOnly::Output PROPERTIES PASS_REGULAR_EXPRESSION "Regex string matcher" FAIL_REGULAR_EXPRESSION "Hidden Test") +add_test(NAME List::Tests::NamesOnly::ExitCode COMMAND $ --list-test-names-only) + + add_test(NAME NoAssertions COMMAND $ -w NoAssertions) set_tests_properties(NoAssertions PROPERTIES PASS_REGULAR_EXPRESSION "No assertions in test case")