From 08939cc8bb01ec84ea01d83cf03e4edb8484460c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sun, 12 Dec 2021 15:18:50 +0100 Subject: [PATCH] Error out early if invalid test specs are provided --- docs/release-notes.md | 1 + src/catch2/catch_session.cpp | 41 +++++++++++++++++++++++------------- tests/CMakeLists.txt | 22 +++++++++++++------ 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 014191b9..ca14092a 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -197,6 +197,7 @@ new design. * Catch2's pkg-config integration also provides 2 packages * `catch2` is the statically compiled implementation by itself * `catch2-with-main` also links in the default main +* Passing invalid test specifications passed to Catch2 are now reported before tests are run, and are a hard error. diff --git a/src/catch2/catch_session.cpp b/src/catch2/catch_session.cpp index ed6479e0..b50d6e48 100644 --- a/src/catch2/catch_session.cpp +++ b/src/catch2/catch_session.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -61,24 +62,30 @@ namespace Catch { m_config{config}, m_context{config, CATCH_MOVE(reporter)} { - auto const& allTestCases = getAllTestCasesSorted(*m_config); - m_matches = m_config->testSpec().matchesByFilter(allTestCases, *m_config); - auto const& invalidArgs = m_config->testSpec().getInvalidArgs(); + assert( m_config->testSpec().getInvalidArgs().empty() && + "Invalid test specs should be handled before running tests" ); - if (m_matches.empty() && invalidArgs.empty()) { - for (auto const& test : allTestCases) - if (!test.getTestCaseInfo().isHidden()) - m_tests.emplace(&test); + auto const& allTestCases = getAllTestCasesSorted(*m_config); + auto const& testSpec = m_config->testSpec(); + if ( !testSpec.hasFilters() ) { + for ( auto const& test : allTestCases ) { + if ( !test.getTestCaseInfo().isHidden() ) { + m_tests.emplace( &test ); + } + } } else { - for (auto const& match : m_matches) - m_tests.insert(match.tests.begin(), match.tests.end()); + m_matches = + testSpec.matchesByFilter( allTestCases, *m_config ); + for ( auto const& match : m_matches ) { + m_tests.insert( match.tests.begin(), + match.tests.end() ); + } } m_tests = createShard(m_tests, m_config->shardCount(), m_config->shardIndex()); } Totals execute() { - auto const& invalidArgs = m_config->testSpec().getInvalidArgs(); Totals totals; for (auto const& testCase : m_tests) { if (!m_context.aborting()) @@ -94,11 +101,6 @@ namespace Catch { } } - if (!invalidArgs.empty()) { - for (auto const& invalidArg: invalidArgs) - m_reporter->reportInvalidArguments(invalidArg); - } - return totals; } @@ -284,6 +286,15 @@ namespace Catch { // Create reporter(s) so we can route listings through them auto reporter = makeReporter(m_config.get()); + auto const& invalidSpecs = m_config->testSpec().getInvalidArgs(); + if ( !invalidSpecs.empty() ) { + for ( auto const& spec : invalidSpecs ) { + reporter->reportInvalidArguments( spec ); + } + return 1; + } + + // Handle list request if (list(*reporter, *m_config)) { return 0; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 47f86ebc..6a7563ec 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -294,14 +294,12 @@ set_tests_properties(TestsInFile::SimpleSpecs PROPERTIES PASS_REGULAR_EXPRESSION add_test(NAME TestsInFile::EscapeSpecialCharacters COMMAND $ "-f ${SELF_TEST_DIR}/Misc/special-characters-in-file.input") set_tests_properties(TestsInFile::EscapeSpecialCharacters PROPERTIES PASS_REGULAR_EXPRESSION "1 assertion in 1 test case") -# CTest does not allow us to create an AND of required regular expressions, -# so we have to split the test into 2 parts and look for parts of the expected -# output separately. add_test(NAME TestsInFile::InvalidTestNames-1 COMMAND $ "-f ${SELF_TEST_DIR}/Misc/invalid-test-names.input") -set_tests_properties(TestsInFile::InvalidTestNames-1 PROPERTIES PASS_REGULAR_EXPRESSION "Invalid Filter: \"Test with special, characters in \\\\\" name\"") - -add_test(NAME TestsInFile::InvalidTestNames-2 COMMAND $ "-f ${SELF_TEST_DIR}/Misc/invalid-test-names.input") -set_tests_properties(TestsInFile::InvalidTestNames-2 PROPERTIES PASS_REGULAR_EXPRESSION "No tests ran") +set_tests_properties(TestsInFile::InvalidTestNames-1 + PROPERTIES + PASS_REGULAR_EXPRESSION "Invalid Filter: \"Test with special, characters in \\\\\" name\"" + FAIL_REGULAR_EXPRESSION "No tests ran" +) add_test(NAME TagAlias COMMAND $ [@tricky] --list-tests) set_tests_properties(TagAlias PROPERTIES @@ -371,6 +369,16 @@ set_tests_properties("Benchmarking::FailureReporting::ShouldFailIsRespected" PASS_REGULAR_EXPRESSION "1 failed as expected" ) +add_test(NAME "ErrorHandling::InvalidTestSpecExitsEarly" + COMMAND + $ "[aa,a]" +) +set_tests_properties("ErrorHandling::InvalidTestSpecExitsEarly" + PROPERTIES + PASS_REGULAR_EXPRESSION "Invalid Filter: \\[aa\,a\\]" + FAIL_REGULAR_EXPRESSION "No tests ran" +) + if (CATCH_USE_VALGRIND) add_test(NAME ValgrindRunTests COMMAND valgrind --leak-check=full --error-exitcode=1 $)