Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve reporting of unmatched filters #1684

Merged
merged 14 commits into from
Aug 6, 2019
Merged
1 change: 1 addition & 0 deletions include/internal/catch_interfaces_testcase.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace Catch {
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 );
Expand Down
84 changes: 49 additions & 35 deletions include/internal/catch_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include <cstdlib>
#include <iomanip>
#include <set>
#include <iterator>

namespace Catch {

Expand Down Expand Up @@ -58,46 +60,53 @@ namespace Catch {
return ret;
}


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);
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);

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));
Expand Down Expand Up @@ -274,7 +283,12 @@ namespace Catch {
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
Expand Down
7 changes: 6 additions & 1 deletion include/internal/catch_test_case_registry_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,13 @@ namespace Catch {
}
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 ) {
Expand Down
2 changes: 2 additions & 0 deletions include/internal/catch_test_case_registry_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ namespace Catch {
struct IConfig;

std::vector<TestCase> sortTests( IConfig const& config, std::vector<TestCase> const& unsortedTestCases );

bool isThrowSafe( TestCase const& testCase, IConfig const& config );
sfranzen marked this conversation as resolved.
Show resolved Hide resolved
bool matchTest( TestCase const& testCase, TestSpec const& testSpec, IConfig const& config );

void enforceNoDuplicateTestCases( std::vector<TestCase> const& functions );
Expand Down
73 changes: 55 additions & 18 deletions include/internal/catch_test_spec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "catch_test_spec.h"
#include "catch_string_manip.h"
#include "catch_interfaces_config.h"

#include <algorithm>
#include <string>
Expand All @@ -15,45 +16,81 @@

namespace Catch {

TestSpec::Pattern::Pattern( std::string const& name )
: m_name( name )
{}

TestSpec::Pattern::~Pattern() = default;
TestSpec::NamePattern::~NamePattern() = default;
TestSpec::TagPattern::~TagPattern() = default;
TestSpec::ExcludedPattern::~ExcludedPattern() = default;

std::string const& TestSpec::Pattern::name() const {
return m_name;
}


TestSpec::NamePattern::NamePattern( std::string const& name )
: m_wildcardPattern( toLower( name ), CaseSensitive::No )
: Pattern( name )
, 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 )
: Pattern( tag )
, 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 {
sfranzen marked this conversation as resolved.
Show resolved Hide resolved
std::string name;
for( auto const& p : m_patterns )
name += p->name() + " ";
name.pop_back();
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;
}

}
26 changes: 19 additions & 7 deletions include/internal/catch_test_spec.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,39 @@

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 );
bool matches( TestCaseInfo const& testCase ) const override;
private:
WildcardPattern m_wildcardPattern;
};

class TagPattern : public Pattern {
public:
TagPattern( std::string const& tag );
virtual ~TagPattern();
explicit TagPattern( std::string const& tag );
bool matches( TestCaseInfo const& testCase ) const override;
private:
std::string m_tag;
};

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;
Expand All @@ -60,11 +64,19 @@ namespace Catch {
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;
Expand Down
14 changes: 12 additions & 2 deletions projects/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,18 @@ set_tests_properties(ListTestNamesOnly PROPERTIES
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 $<TARGET_FILE:SelfTest> -w NoTests "___nonexistent_test___")
set_tests_properties(WarnAboutNoTests PROPERTIES
WILL_FAIL TRUE
# Failure is now expected, so test errors must be indicated as passing it
PASS_REGULAR_EXPRESSION "Helper failed with"
sfranzen marked this conversation as resolved.
Show resolved Hide resolved
)

add_test(NAME FilteredSection-1 COMMAND $<TARGET_FILE:SelfTest> \#1394 -c RunSection)
set_tests_properties(FilteredSection-1 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran")
Expand Down