Fixes #1766: Catch terminates when parsing invalid test name

include/internal/catch_commandline.cpp

@@ -49,9 +49,15 @@ namespace Catch {
                     if( !line.empty() && !startsWith( line, '#' ) ) {
                         if( !startsWith( line, '"' ) )
                             line = '"' + line + '"';
-                        config.testsOrTags.push_back( line + ',' );
+                        config.testsOrTags.push_back( line );
+                        config.testsOrTags.push_back( "," );
+                        
                     }
                 }
+                //Remove comma in the end
+                if(!config.testsOrTags.empty())
+                    config.testsOrTags.erase( config.testsOrTags.end()-1 );
+                
                 return ParserResult::ok( ParseResultType::Matched );
             };
         auto const setTestOrder = [&]( std::string const& order ) {

include/internal/catch_interfaces_reporter.h

@@ -214,6 +214,8 @@ namespace Catch {
 
         virtual void noMatchingTestCases( std::string const& spec ) = 0;
 
+        virtual void reportInvalidArguments(std::string const&) {}
+        
         virtual void testRunStarting( TestRunInfo const& testRunInfo ) = 0;
         virtual void testGroupStarting( GroupInfo const& groupInfo ) = 0;
 

include/internal/catch_session.cpp

@@ -68,8 +68,9 @@ namespace Catch {
             {
                 auto const& allTestCases = getAllTestCasesSorted(*m_config);
                 m_matches = m_config->testSpec().matchesByFilter(allTestCases, *m_config);
+                auto const& invalidArgs = m_config->testSpec().getInvalidArgs();
                 
-                if (m_matches.empty()) {
+                if (m_matches.empty() && invalidArgs.empty()) {
                     for (auto const& test : allTestCases)
                         if (!test.isHidden())
                             m_tests.emplace(&test);
@@ -80,6 +81,7 @@ namespace Catch {
             }
 
             Totals execute() {
+                auto const& invalidArgs = m_config->testSpec().getInvalidArgs();
                 Totals totals;
                 m_context.testGroupStarting(m_config->name(), 1, 1);
                 for (auto const& testCase : m_tests) {
@@ -95,6 +97,12 @@ namespace Catch {
                         totals.error = -1;
                     }
                 }
+                
+                if (!invalidArgs.empty()) {
+                    for (auto const& invalidArg: invalidArgs)                   
+                         m_context.reporter().reportInvalidArguments(invalidArg);
+                }   
+                
                 m_context.testGroupEnded(m_config->name(), totals, 1, 1);
                 return totals;
             }

include/internal/catch_test_spec.cpp

@@ -92,4 +92,8 @@ namespace Catch {
         return matches;
     }
     
+    const TestSpec::vectorStrings& TestSpec::getInvalidArgs() const{
+        return  (m_invalidArgs);
+    }
+
 }

include/internal/catch_test_spec.h

@@ -73,14 +73,16 @@ namespace Catch {
             std::vector<TestCase const*> tests;
         };
         using Matches = std::vector<FilterMatch>;
+        using vectorStrings = std::vector<std::string>;
 
         bool hasFilters() const;
         bool matches( TestCaseInfo const& testCase ) const;
         Matches matchesByFilter( std::vector<TestCase> const& testCases, IConfig const& config ) const;
+        const vectorStrings & getInvalidArgs() const;
 
     private:
         std::vector<Filter> m_filters;
-
+        std::vector<std::string> m_invalidArgs;
         friend class TestSpecParser;
     };
 }

include/internal/catch_test_spec_parser.cpp

@@ -20,8 +20,13 @@ namespace Catch {
         m_substring.reserve(m_arg.size());
         m_patternName.reserve(m_arg.size());
         m_realPatternPos = 0;
+        
         for( m_pos = 0; m_pos < m_arg.size(); ++m_pos )
-            visitChar( m_arg[m_pos] );
+          //if visitChar fails
+           if( !visitChar( m_arg[m_pos] ) ){ 
+               m_testSpec.m_invalidArgs.push_back(arg);
+               break;
+           }
         endMode();
         return *this;
     }
@@ -29,38 +34,32 @@ namespace Catch {
         addFilter();
         return m_testSpec;
     }
-    void TestSpecParser::visitChar( char c ) {
+    bool TestSpecParser::visitChar( char c ) {
         if( (m_mode != EscapedName) && (c == '\\') ) {
             escape();
-            m_substring += c;
+            addCharToPattern(c);
-            m_patternName += c;
+            return true;
-            m_realPatternPos++;
-            return;
         }else if((m_mode != EscapedName) && (c == ',') )  {
-            endMode();
+            return separate();
-            addFilter();
-            return;
         }
 
         switch( m_mode ) {
         case None:
             if( processNoneChar( c ) )
-                return;
+                return true;
             break;
         case Name:
             processNameChar( c );
             break;
         case EscapedName:
             endMode();
-            m_substring += c;
+            addCharToPattern(c);
-            m_patternName += c;
+            return true;
-            m_realPatternPos++;
-            return;
         default:
         case Tag:
         case QuotedName:
             if( processOtherChar( c ) )
-                return;
+                return true;
             break;
         }
 
@@ -69,6 +68,7 @@ namespace Catch {
             m_patternName += c;
             m_realPatternPos++;
         }
+        return true;
     }
     // Two of the processing methods return true to signal the caller to return
     // without adding the given character to the current pattern strings
@@ -161,6 +161,20 @@ namespace Catch {
       m_mode = lastMode;
     }
     
+    bool TestSpecParser::separate() {  
+      if( (m_mode==QuotedName) || (m_mode==Tag) ){
+         //invalid argument, signal failure to previous scope.
+         m_mode = None;
+         m_pos = m_arg.size();
+         m_substring.clear();
+         m_patternName.clear();
+         return false;
+      }
+      endMode();
+      addFilter();
+      return true; //success
+    }
+    
     TestSpec parseTestSpec( std::string const& arg ) {
         return TestSpecParser( ITagAliasRegistry::get() ).parse( arg ).testSpec();
     }

include/internal/catch_test_spec_parser.h

@@ -41,7 +41,7 @@ namespace Catch {
         TestSpec testSpec();
 
     private:
-        void visitChar( char c );
+        bool visitChar( char c );
         void startNewMode( Mode mode );
         bool processNoneChar( char c );
         void processNameChar( char c );
@@ -51,6 +51,8 @@ namespace Catch {
         bool isControlChar( char c ) const;
         void saveLastMode();
         void revertBackToLastMode();
+        void addFilter();
+        bool separate();
         
         template<typename T>
         void addPattern() {
@@ -74,7 +76,12 @@ namespace Catch {
             m_mode = None;
         }
         
-        void addFilter();
+        inline void addCharToPattern(char c) {
+            m_substring += c;
+            m_patternName += c;
+            m_realPatternPos++;
+        }
+        
     };
     TestSpec parseTestSpec( std::string const& arg );
 

include/reporters/catch_reporter_bases.hpp

@@ -51,6 +51,8 @@ namespace Catch {
 
         void noMatchingTestCases(std::string const&) override {}
 
+        void reportInvalidArguments(std::string const&) override {}
+        
         void testRunStarting(TestRunInfo const& _testRunInfo) override {
             currentTestRunInfo = _testRunInfo;
         }

include/reporters/catch_reporter_console.cpp

@@ -383,6 +383,10 @@ void ConsoleReporter::noMatchingTestCases(std::string const& spec) {
     stream << "No test cases matched '" << spec << '\'' << std::endl;
 }
 
+void ConsoleReporter::reportInvalidArguments(std::string const&arg){
+    stream << "Invalid Filter: " << arg << std::endl;
+}
+
 void ConsoleReporter::assertionStarting(AssertionInfo const&) {}
 
 bool ConsoleReporter::assertionEnded(AssertionStats const& _assertionStats) {

include/reporters/catch_reporter_console.h

@@ -32,6 +32,8 @@ namespace Catch {
 
         void noMatchingTestCases(std::string const& spec) override;
 
+        void reportInvalidArguments(std::string const&arg) override;
+
         void assertionStarting(AssertionInfo const&) override;
 
         bool assertionEnded(AssertionStats const& _assertionStats) override;

include/reporters/catch_reporter_listening.cpp

@@ -42,6 +42,13 @@ namespace Catch {
         m_reporter->noMatchingTestCases( spec );
     }
     
+    void ListeningReporter::reportInvalidArguments(std::string const&arg){
+        for ( auto const& listener : m_listeners ) {
+            listener->reportInvalidArguments( arg );
+        }
+        m_reporter->reportInvalidArguments( arg );
+    }    
+
 #if defined(CATCH_CONFIG_ENABLE_BENCHMARKING)
     void ListeningReporter::benchmarkPreparing( std::string const& name ) {
 		for (auto const& listener : m_listeners) {

include/reporters/catch_reporter_listening.h

@@ -29,6 +29,8 @@ namespace Catch {
 
         void noMatchingTestCases( std::string const& spec ) override;
         
+        void reportInvalidArguments(std::string const&arg) override;
+        
         static std::set<Verbosity> getSupportedVerbosities();
 
 #if defined(CATCH_CONFIG_ENABLE_BENCHMARKING)