Skip to content

Commit

Permalink
Make --list-* exit code be 0
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@horenmar
horenmar committed Oct 28, 2019
1 parent 8128c09 commit 2c06ee9
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 138 deletions.
2 changes: 2 additions & 0 deletions docs/release-notes.md
View file
Expand Up @@ -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

Expand Down
231 changes: 124 additions & 107 deletions include/internal/catch_list.cpp
View file
Expand Up @@ -24,59 +24,125 @@
#include <limits>
#include <algorithm>
#include <iomanip>
#include <set>

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<std::string> 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<TestCase> 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<TestCase> 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<std::string, TagInfo> tagCounts;

std::vector<TestCase> 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;
Expand All @@ -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<Config> const& config ) {
bool listed = false;
getCurrentMutableContext().setConfig( config );
if (config->listTests()) {
listed = true;
listTests(*config);
}

std::map<std::string, TagInfo> tagCounts;

std::vector<TestCase> 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<std::size_t> list( std::shared_ptr<Config> const& config ) {
Option<std::size_t> 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
21 changes: 1 addition & 20 deletions include/internal/catch_list.h
View file
Expand Up @@ -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 <set>

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<std::string> spellings;
std::size_t count = 0;
};

std::size_t listTags( Config const& config );

std::size_t listReporters();

Option<std::size_t> list( std::shared_ptr<Config> const& config );
bool list( std::shared_ptr<Config> const& config );

} // end namespace Catch

Expand Down
5 changes: 3 additions & 2 deletions include/internal/catch_session.cpp
View file
Expand Up @@ -288,8 +288,9 @@ namespace Catch {
applyFilenamesAsTags( *m_config );

// Handle list request
if( Option<std::size_t> listed = list( m_config ) )
return static_cast<int>( *listed );
if (list(m_config)) {
return 0;
}

TestGroup tests { m_config };
auto const totals = tests.execute();
Expand Down
27 changes: 18 additions & 9 deletions projects/CMakeLists.txt
View file
Expand Up @@ -376,24 +376,33 @@ set_tests_properties(RunTests PROPERTIES
FAIL_REGULAR_EXPRESSION "Filters:"
)

add_test(NAME ListTests COMMAND $<TARGET_FILE:SelfTest> --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 $<TARGET_FILE:SelfTest> --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 $<TARGET_FILE:SelfTest> --list-tests --verbosity high)

add_test(NAME ListTags COMMAND $<TARGET_FILE:SelfTest> --list-tags)
set_tests_properties(ListTags PROPERTIES
PASS_REGULAR_EXPRESSION "[0-9]+ tags"
add_test(NAME List::Tags::Output COMMAND $<TARGET_FILE:SelfTest> --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 $<TARGET_FILE:SelfTest> --list-tags)

add_test(NAME ListReporters COMMAND $<TARGET_FILE:SelfTest> --list-reporters)
set_tests_properties(ListReporters PROPERTIES PASS_REGULAR_EXPRESSION "Available reporters:")
add_test(NAME List::Reporters::Output COMMAND $<TARGET_FILE:SelfTest> --list-reporters)
set_tests_properties(List::Reporters::Output PROPERTIES PASS_REGULAR_EXPRESSION "Available reporters:")
add_test(NAME List::Reporters::ExitCode COMMAND $<TARGET_FILE:SelfTest> --list-reporters)

add_test(NAME ListTestNamesOnly COMMAND $<TARGET_FILE:SelfTest> --list-test-names-only)
set_tests_properties(ListTestNamesOnly PROPERTIES
add_test(NAME List::Tests::NamesOnly::Output COMMAND $<TARGET_FILE:SelfTest> --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 $<TARGET_FILE:SelfTest> --list-test-names-only)



add_test(NAME NoAssertions COMMAND $<TARGET_FILE:SelfTest> -w NoAssertions)
set_tests_properties(NoAssertions PROPERTIES PASS_REGULAR_EXPRESSION "No assertions in test case")
Expand Down

0 comments on commit 2c06ee9

Please sign in to comment.