Improve reporting of unmatched filters (#1684)
This PR ultimately does 3 things
* Separately tracks matched tests per each filter part (that is, a set of filters separated by an OR (`,`)), which allows Catch2 to report each of the alternative filters that don't match any tests.
* Fixes `-w NoTests` to return non-zero in the process
* Adds tests for `-w NoTests`.
diff --git a/include/internal/catch_interfaces_testcase.h b/include/internal/catch_interfaces_testcase.h
index f57cc8f..2492c07 100644
--- a/include/internal/catch_interfaces_testcase.h
+++ b/include/internal/catch_interfaces_testcase.h
@@ -28,6 +28,7 @@
virtual std::vector<TestCase> const& getAllTestsSorted( IConfig const& config ) const = 0;
};
+ bool isThrowSafe( TestCase const& testCase, IConfig const& config );
bool matchTest( TestCase const& testCase, TestSpec const& testSpec, IConfig const& config );
std::vector<TestCase> filterTests( std::vector<TestCase> const& testCases, TestSpec const& testSpec, IConfig const& config );
std::vector<TestCase> const& getAllTestCasesSorted( IConfig const& config );
diff --git a/include/internal/catch_session.cpp b/include/internal/catch_session.cpp
index f9818f7..13aaeec 100644
--- a/include/internal/catch_session.cpp
+++ b/include/internal/catch_session.cpp
@@ -25,6 +25,8 @@
#include <cstdlib>
#include <iomanip>
+#include <set>
+#include <iterator>
namespace Catch {
@@ -58,46 +60,53 @@
return ret;
}
+ class TestGroup {
+ public:
+ explicit TestGroup(std::shared_ptr<Config> const& config)
+ : m_config{config}
+ , m_context{config, makeReporter(config)}
+ {
+ auto const& allTestCases = getAllTestCasesSorted(*m_config);
+ m_matches = m_config->testSpec().matchesByFilter(allTestCases, *m_config);
- Catch::Totals runTests(std::shared_ptr<Config> const& config) {
- auto reporter = makeReporter(config);
-
- RunContext context(config, std::move(reporter));
-
- Totals totals;
-
- context.testGroupStarting(config->name(), 1, 1);
-
- TestSpec testSpec = config->testSpec();
-
- auto const& allTestCases = getAllTestCasesSorted(*config);
- for (auto const& testCase : allTestCases) {
- bool matching = (!testSpec.hasFilters() && !testCase.isHidden()) ||
- (testSpec.hasFilters() && matchTest(testCase, testSpec, *config));
-
- if (!context.aborting() && matching)
- totals += context.runTest(testCase);
- else
- context.reporter().skipTest(testCase);
+ if (m_matches.empty()) {
+ for (auto const& test : allTestCases)
+ if (!test.isHidden())
+ m_tests.emplace(&test);
+ } else {
+ for (auto const& match : m_matches)
+ m_tests.insert(match.tests.begin(), match.tests.end());
+ }
}
- if (config->warnAboutNoTests() && totals.testCases.total() == 0) {
- ReusableStringStream testConfig;
-
- bool first = true;
- for (const auto& input : config->getTestsOrTags()) {
- if (!first) { testConfig << ' '; }
- first = false;
- testConfig << input;
+ Totals execute() {
+ Totals totals;
+ m_context.testGroupStarting(m_config->name(), 1, 1);
+ for (auto const& testCase : m_tests) {
+ if (!m_context.aborting())
+ totals += m_context.runTest(*testCase);
+ else
+ m_context.reporter().skipTest(*testCase);
}
- context.reporter().noMatchingTestCases(testConfig.str());
- totals.error = -1;
+ for (auto const& match : m_matches) {
+ if (match.tests.empty()) {
+ m_context.reporter().noMatchingTestCases(match.name);
+ totals.error = -1;
+ }
+ }
+ m_context.testGroupEnded(m_config->name(), totals, 1, 1);
+ return totals;
}
- context.testGroupEnded(config->name(), totals, 1, 1);
- return totals;
- }
+ private:
+ using Tests = std::set<TestCase const*>;
+
+ std::shared_ptr<Config> m_config;
+ RunContext m_context;
+ Tests m_tests;
+ TestSpec::Matches m_matches;
+ };
void applyFilenamesAsTags(Catch::IConfig const& config) {
auto& tests = const_cast<std::vector<TestCase>&>(getAllTestCasesSorted(config));
@@ -274,7 +283,12 @@
if( Option<std::size_t> listed = list( m_config ) )
return static_cast<int>( *listed );
- auto totals = runTests( m_config );
+ TestGroup tests { m_config };
+ auto const totals = tests.execute();
+
+ if( m_config->warnAboutNoTests() && totals.error == -1 )
+ return 2;
+
// Note that on unices only the lower 8 bits are usually used, clamping
// the return value to 255 prevents false negative when some multiple
// of 256 tests has failed
diff --git a/include/internal/catch_test_case_registry_impl.cpp b/include/internal/catch_test_case_registry_impl.cpp
index a85d0ed..bb59680 100644
--- a/include/internal/catch_test_case_registry_impl.cpp
+++ b/include/internal/catch_test_case_registry_impl.cpp
@@ -36,8 +36,13 @@
}
return sorted;
}
+
+ bool isThrowSafe( TestCase const& testCase, IConfig const& config ) {
+ return !testCase.throws() || config.allowThrows();
+ }
+
bool matchTest( TestCase const& testCase, TestSpec const& testSpec, IConfig const& config ) {
- return testSpec.matches( testCase ) && ( config.allowThrows() || !testCase.throws() );
+ return testSpec.matches( testCase ) && isThrowSafe( testCase, config );
}
void enforceNoDuplicateTestCases( std::vector<TestCase> const& functions ) {
diff --git a/include/internal/catch_test_case_registry_impl.h b/include/internal/catch_test_case_registry_impl.h
index 8dc5b0f..359ac3e 100644
--- a/include/internal/catch_test_case_registry_impl.h
+++ b/include/internal/catch_test_case_registry_impl.h
@@ -23,6 +23,8 @@
struct IConfig;
std::vector<TestCase> sortTests( IConfig const& config, std::vector<TestCase> const& unsortedTestCases );
+
+ bool isThrowSafe( TestCase const& testCase, IConfig const& config );
bool matchTest( TestCase const& testCase, TestSpec const& testSpec, IConfig const& config );
void enforceNoDuplicateTestCases( std::vector<TestCase> const& functions );
diff --git a/include/internal/catch_test_spec.cpp b/include/internal/catch_test_spec.cpp
index d9c149d..54b638c 100644
--- a/include/internal/catch_test_spec.cpp
+++ b/include/internal/catch_test_spec.cpp
@@ -7,6 +7,7 @@
#include "catch_test_spec.h"
#include "catch_string_manip.h"
+#include "catch_interfaces_config.h"
#include <algorithm>
#include <string>
@@ -15,45 +16,80 @@
namespace Catch {
- TestSpec::Pattern::~Pattern() = default;
- TestSpec::NamePattern::~NamePattern() = default;
- TestSpec::TagPattern::~TagPattern() = default;
- TestSpec::ExcludedPattern::~ExcludedPattern() = default;
-
- TestSpec::NamePattern::NamePattern( std::string const& name )
- : m_wildcardPattern( toLower( name ), CaseSensitive::No )
+ TestSpec::Pattern::Pattern( std::string const& name )
+ : m_name( name )
{}
+
+ TestSpec::Pattern::~Pattern() = default;
+
+ std::string const& TestSpec::Pattern::name() const {
+ return m_name;
+ }
+
+
+ TestSpec::NamePattern::NamePattern( std::string const& name, std::string const& filterString )
+ : Pattern( filterString )
+ , m_wildcardPattern( toLower( name ), CaseSensitive::No )
+ {}
+
bool TestSpec::NamePattern::matches( TestCaseInfo const& testCase ) const {
return m_wildcardPattern.matches( toLower( testCase.name ) );
}
- TestSpec::TagPattern::TagPattern( std::string const& tag ) : m_tag( toLower( tag ) ) {}
+
+ TestSpec::TagPattern::TagPattern( std::string const& tag, std::string const& filterString )
+ : Pattern( filterString )
+ , m_tag( toLower( tag ) )
+ {}
+
bool TestSpec::TagPattern::matches( TestCaseInfo const& testCase ) const {
return std::find(begin(testCase.lcaseTags),
end(testCase.lcaseTags),
m_tag) != end(testCase.lcaseTags);
}
- TestSpec::ExcludedPattern::ExcludedPattern( PatternPtr const& underlyingPattern ) : m_underlyingPattern( underlyingPattern ) {}
- bool TestSpec::ExcludedPattern::matches( TestCaseInfo const& testCase ) const { return !m_underlyingPattern->matches( testCase ); }
+
+ TestSpec::ExcludedPattern::ExcludedPattern( PatternPtr const& underlyingPattern )
+ : Pattern( underlyingPattern->name() )
+ , m_underlyingPattern( underlyingPattern )
+ {}
+
+ bool TestSpec::ExcludedPattern::matches( TestCaseInfo const& testCase ) const {
+ return !m_underlyingPattern->matches( testCase );
+ }
+
bool TestSpec::Filter::matches( TestCaseInfo const& testCase ) const {
- // All patterns in a filter must match for the filter to be a match
- for( auto const& pattern : m_patterns ) {
- if( !pattern->matches( testCase ) )
- return false;
- }
- return true;
+ return std::all_of( m_patterns.begin(), m_patterns.end(), [&]( PatternPtr const& p ){ return p->matches( testCase ); } );
}
+ std::string TestSpec::Filter::name() const {
+ std::string name;
+ for( auto const& p : m_patterns )
+ name += p->name();
+ return name;
+ }
+
+
bool TestSpec::hasFilters() const {
return !m_filters.empty();
}
+
bool TestSpec::matches( TestCaseInfo const& testCase ) const {
- // A TestSpec matches if any filter matches
- for( auto const& filter : m_filters )
- if( filter.matches( testCase ) )
- return true;
- return false;
+ return std::any_of( m_filters.begin(), m_filters.end(), [&]( Filter const& f ){ return f.matches( testCase ); } );
}
+
+ TestSpec::Matches TestSpec::matchesByFilter( std::vector<TestCase> const& testCases, IConfig const& config ) const
+ {
+ Matches matches( m_filters.size() );
+ std::transform( m_filters.begin(), m_filters.end(), matches.begin(), [&]( Filter const& filter ){
+ std::vector<TestCase const*> currentMatches;
+ for( auto const& test : testCases )
+ if( isThrowSafe( test, config ) && filter.matches( test ) )
+ currentMatches.emplace_back( &test );
+ return FilterMatch{ filter.name(), currentMatches };
+ } );
+ return matches;
+ }
+
}
diff --git a/include/internal/catch_test_spec.h b/include/internal/catch_test_spec.h
index d256518..d0e7ea9 100644
--- a/include/internal/catch_test_spec.h
+++ b/include/internal/catch_test_spec.h
@@ -22,17 +22,23 @@
namespace Catch {
+ struct IConfig;
+
class TestSpec {
- struct Pattern {
+ class Pattern {
+ public:
+ explicit Pattern( std::string const& name );
virtual ~Pattern();
virtual bool matches( TestCaseInfo const& testCase ) const = 0;
+ std::string const& name() const;
+ private:
+ std::string const m_name;
};
using PatternPtr = std::shared_ptr<Pattern>;
class NamePattern : public Pattern {
public:
- NamePattern( std::string const& name );
- virtual ~NamePattern();
+ explicit NamePattern( std::string const& name, std::string const& filterString );
bool matches( TestCaseInfo const& testCase ) const override;
private:
WildcardPattern m_wildcardPattern;
@@ -40,8 +46,7 @@
class TagPattern : public Pattern {
public:
- TagPattern( std::string const& tag );
- virtual ~TagPattern();
+ explicit TagPattern( std::string const& tag, std::string const& filterString );
bool matches( TestCaseInfo const& testCase ) const override;
private:
std::string m_tag;
@@ -49,8 +54,7 @@
class ExcludedPattern : public Pattern {
public:
- ExcludedPattern( PatternPtr const& underlyingPattern );
- virtual ~ExcludedPattern();
+ explicit ExcludedPattern( PatternPtr const& underlyingPattern );
bool matches( TestCaseInfo const& testCase ) const override;
private:
PatternPtr m_underlyingPattern;
@@ -60,11 +64,19 @@
std::vector<PatternPtr> m_patterns;
bool matches( TestCaseInfo const& testCase ) const;
+ std::string name() const;
};
public:
+ struct FilterMatch {
+ std::string name;
+ std::vector<TestCase const*> tests;
+ };
+ using Matches = std::vector<FilterMatch>;
+
bool hasFilters() const;
bool matches( TestCaseInfo const& testCase ) const;
+ Matches matchesByFilter( std::vector<TestCase> const& testCases, IConfig const& config ) const;
private:
std::vector<Filter> m_filters;
diff --git a/include/internal/catch_test_spec_parser.cpp b/include/internal/catch_test_spec_parser.cpp
index 61c9e4d..a910ac7 100644
--- a/include/internal/catch_test_spec_parser.cpp
+++ b/include/internal/catch_test_spec_parser.cpp
@@ -14,64 +14,125 @@
TestSpecParser& TestSpecParser::parse( std::string const& arg ) {
m_mode = None;
m_exclusion = false;
- m_start = std::string::npos;
m_arg = m_tagAliases->expandAliases( arg );
m_escapeChars.clear();
+ m_substring.reserve(m_arg.size());
+ m_patternName.reserve(m_arg.size());
for( m_pos = 0; m_pos < m_arg.size(); ++m_pos )
visitChar( m_arg[m_pos] );
- if( m_mode == Name )
- addPattern<TestSpec::NamePattern>();
+ endMode();
return *this;
}
TestSpec TestSpecParser::testSpec() {
addFilter();
return m_testSpec;
}
-
void TestSpecParser::visitChar( char c ) {
- if( m_mode == None ) {
- switch( c ) {
- case ' ': return;
- case '~': m_exclusion = true; return;
- case '[': return startNewMode( Tag, ++m_pos );
- case '"': return startNewMode( QuotedName, ++m_pos );
- case '\\': return escape();
- default: startNewMode( Name, m_pos ); break;
- }
+ if( c == ',' ) {
+ endMode();
+ addFilter();
+ return;
}
- if( m_mode == Name ) {
- if( c == ',' ) {
- addPattern<TestSpec::NamePattern>();
- addFilter();
- }
- else if( c == '[' ) {
- if( subString() == "exclude:" )
- m_exclusion = true;
- else
- addPattern<TestSpec::NamePattern>();
- startNewMode( Tag, ++m_pos );
- }
- else if( c == '\\' )
- escape();
+
+ switch( m_mode ) {
+ case None:
+ if( processNoneChar( c ) )
+ return;
+ break;
+ case Name:
+ processNameChar( c );
+ break;
+ case EscapedName:
+ endMode();
+ break;
+ default:
+ case Tag:
+ case QuotedName:
+ if( processOtherChar( c ) )
+ return;
+ break;
}
- else if( m_mode == EscapedName )
- m_mode = Name;
- else if( m_mode == QuotedName && c == '"' )
- addPattern<TestSpec::NamePattern>();
- else if( m_mode == Tag && c == ']' )
- addPattern<TestSpec::TagPattern>();
+
+ m_substring += c;
+ if( !isControlChar( c ) )
+ m_patternName += c;
}
- void TestSpecParser::startNewMode( Mode mode, std::size_t start ) {
+ // Two of the processing methods return true to signal the caller to return
+ // without adding the given character to the current pattern strings
+ bool TestSpecParser::processNoneChar( char c ) {
+ switch( c ) {
+ case ' ':
+ return true;
+ case '~':
+ m_exclusion = true;
+ return false;
+ case '[':
+ startNewMode( Tag );
+ return false;
+ case '"':
+ startNewMode( QuotedName );
+ return false;
+ case '\\':
+ escape();
+ return true;
+ default:
+ startNewMode( Name );
+ return false;
+ }
+ }
+ void TestSpecParser::processNameChar( char c ) {
+ if( c == '[' ) {
+ if( m_substring == "exclude:" )
+ m_exclusion = true;
+ else
+ endMode();
+ startNewMode( Tag );
+ }
+ }
+ bool TestSpecParser::processOtherChar( char c ) {
+ if( !isControlChar( c ) )
+ return false;
+ m_substring += c;
+ endMode();
+ return true;
+ }
+ void TestSpecParser::startNewMode( Mode mode ) {
m_mode = mode;
- m_start = start;
+ }
+ void TestSpecParser::endMode() {
+ switch( m_mode ) {
+ case Name:
+ case QuotedName:
+ return addPattern<TestSpec::NamePattern>();
+ case Tag:
+ return addPattern<TestSpec::TagPattern>();
+ case EscapedName:
+ return startNewMode( Name );
+ case None:
+ default:
+ return startNewMode( None );
+ }
}
void TestSpecParser::escape() {
- if( m_mode == None )
- m_start = m_pos;
m_mode = EscapedName;
m_escapeChars.push_back( m_pos );
}
- std::string TestSpecParser::subString() const { return m_arg.substr( m_start, m_pos - m_start ); }
+ bool TestSpecParser::isControlChar( char c ) const {
+ switch( m_mode ) {
+ default:
+ return false;
+ case None:
+ return c == '~';
+ case Name:
+ return c == '[';
+ case EscapedName:
+ return true;
+ case QuotedName:
+ return c == '"';
+ case Tag:
+ return c == '[' || c == ']';
+ }
+ }
void TestSpecParser::addFilter() {
if( !m_currentFilter.m_patterns.empty() ) {
diff --git a/include/internal/catch_test_spec_parser.h b/include/internal/catch_test_spec_parser.h
index 79ce889..5b02bf6 100644
--- a/include/internal/catch_test_spec_parser.h
+++ b/include/internal/catch_test_spec_parser.h
@@ -23,8 +23,10 @@
enum Mode{ None, Name, QuotedName, Tag, EscapedName };
Mode m_mode = None;
bool m_exclusion = false;
- std::size_t m_start = std::string::npos, m_pos = 0;
+ std::size_t m_pos = 0;
std::string m_arg;
+ std::string m_substring;
+ std::string m_patternName;
std::vector<std::size_t> m_escapeChars;
TestSpec::Filter m_currentFilter;
TestSpec m_testSpec;
@@ -38,26 +40,32 @@
private:
void visitChar( char c );
- void startNewMode( Mode mode, std::size_t start );
+ void startNewMode( Mode mode );
+ bool processNoneChar( char c );
+ void processNameChar( char c );
+ bool processOtherChar( char c );
+ void endMode();
void escape();
- std::string subString() const;
+ bool isControlChar( char c ) const;
template<typename T>
void addPattern() {
- std::string token = subString();
+ std::string token = m_patternName;
for( std::size_t i = 0; i < m_escapeChars.size(); ++i )
- token = token.substr( 0, m_escapeChars[i]-m_start-i ) + token.substr( m_escapeChars[i]-m_start-i+1 );
+ token = token.substr( 0, m_escapeChars[i] - i ) + token.substr( m_escapeChars[i] -i +1 );
m_escapeChars.clear();
if( startsWith( token, "exclude:" ) ) {
m_exclusion = true;
token = token.substr( 8 );
}
if( !token.empty() ) {
- TestSpec::PatternPtr pattern = std::make_shared<T>( token );
+ TestSpec::PatternPtr pattern = std::make_shared<T>( token, m_substring );
if( m_exclusion )
pattern = std::make_shared<TestSpec::ExcludedPattern>( pattern );
m_currentFilter.m_patterns.push_back( pattern );
}
+ m_substring.clear();
+ m_patternName.clear();
m_exclusion = false;
m_mode = None;
}
diff --git a/projects/CMakeLists.txt b/projects/CMakeLists.txt
index cbaaa58..94233d7 100644
--- a/projects/CMakeLists.txt
+++ b/projects/CMakeLists.txt
@@ -393,8 +393,19 @@
add_test(NAME NoAssertions COMMAND $<TARGET_FILE:SelfTest> -w NoAssertions)
set_tests_properties(NoAssertions PROPERTIES PASS_REGULAR_EXPRESSION "No assertions in test case")
-add_test(NAME NoTest COMMAND $<TARGET_FILE:SelfTest> -w NoTests "___nonexistent_test___")
-set_tests_properties(NoTest PROPERTIES PASS_REGULAR_EXPRESSION "No test cases matched")
+add_test(NAME NoTest COMMAND $<TARGET_FILE:SelfTest> Tracker, "___nonexistent_test___")
+set_tests_properties(NoTest PROPERTIES
+ PASS_REGULAR_EXPRESSION "No test cases matched '___nonexistent_test___'"
+ FAIL_REGULAR_EXPRESSION "No tests ran"
+)
+
+add_test(NAME WarnAboutNoTests COMMAND ${CMAKE_COMMAND} -P ${CATCH_DIR}/projects/SelfTest/WarnAboutNoTests.cmake $<TARGET_FILE:SelfTest>)
+
+add_test(NAME UnmatchedOutputFilter COMMAND $<TARGET_FILE:SelfTest> [this-tag-does-not-exist] -w NoTests)
+set_tests_properties(UnmatchedOutputFilter
+ PROPERTIES
+ PASS_REGULAR_EXPRESSION "No test cases matched '\\[this-tag-does-not-exist\\]'"
+)
add_test(NAME FilteredSection-1 COMMAND $<TARGET_FILE:SelfTest> \#1394 -c RunSection)
set_tests_properties(FilteredSection-1 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran")
diff --git a/projects/SelfTest/WarnAboutNoTests.cmake b/projects/SelfTest/WarnAboutNoTests.cmake
new file mode 100644
index 0000000..4637e3f
--- /dev/null
+++ b/projects/SelfTest/WarnAboutNoTests.cmake
@@ -0,0 +1,19 @@
+# Workaround for a peculiarity where CTest disregards the return code from a
+# test command if a PASS_REGULAR_EXPRESSION is also set
+execute_process(
+ COMMAND ${CMAKE_ARGV3} -w NoTests "___nonexistent_test___"
+ RESULT_VARIABLE ret
+ OUTPUT_VARIABLE out
+)
+
+message("${out}")
+
+if(NOT ${ret} MATCHES "^[0-9]+$")
+ message(FATAL_ERROR "${ret}")
+endif()
+
+if(${ret} EQUAL 0)
+ message(FATAL_ERROR "Expected nonzero return code")
+elseif(${out} MATCHES "Helper failed with")
+ message(FATAL_ERROR "Helper failed")
+endif()
