Improve error messages for TEST_CASE tag parsing errors

Also removes a duplicated test case checking for empty tag error.

Related to #2650

src/catch2/catch_test_case_info.cpp

@@ -139,12 +139,20 @@ namespace Catch {
         for (size_t idx = 0; idx < originalTags.size(); ++idx) {
             auto c = originalTags[idx];
             if (c == '[') {
-                assert(!inTag);
+                CATCH_ENFORCE(
+                    !inTag,
+                    "Found '[' inside a tag while registering test case '"
+                        << _nameAndTags.name << "' at " << _lineInfo );
+
                 inTag = true;
                 tagStart = idx;
             }
             if (c == ']') {
-                assert(inTag);
+                CATCH_ENFORCE(
+                    inTag,
+                    "Found unmatched ']' while registering test case '"
+                        << _nameAndTags.name << "' at " << _lineInfo );
+
                 inTag = false;
                 tagEnd = idx;
                 assert(tagStart < tagEnd);
@@ -153,7 +161,11 @@ namespace Catch {
                 // it over to backing storage and actually reference the
                 // backing storage in the saved tags
                 StringRef tagStr = originalTags.substr(tagStart+1, tagEnd - tagStart - 1);
-                CATCH_ENFORCE(!tagStr.empty(), "Empty tags are not allowed");
+                CATCH_ENFORCE( !tagStr.empty(),
+                               "Found an empty tag while registering test case '"
+                                   << _nameAndTags.name << "' at "
+                                   << _lineInfo );
+
                 enforceNotReservedTag(tagStr, lineInfo);
                 properties |= parseSpecialTag(tagStr);
                 // When copying a tag to the backing storage, we need to
@@ -167,8 +179,12 @@ namespace Catch {
                 // the tags.
                 internalAppendTag(tagStr);
             }
-            (void)inTag; // Silence "set-but-unused" warning in release mode.
         }
+        CATCH_ENFORCE( !inTag,
+                       "Found an unclosed tag while registering test case '"
+                           << _nameAndTags.name << "' at " << _lineInfo );
+
+
         // Add [.] if relevant
         if (isHidden()) {
             internalAppendTag("."_sr);

tests/SelfTest/Baselines/automake.sw.approved.txt

@@ -131,7 +131,6 @@ Nor would this
 :test-result: PASS Default scale is invisible to comparison
 :test-result: PASS Directly creating an EnumInfo
 :test-result: PASS Empty stream name opens cout stream
-:test-result: PASS Empty tag is not allowed
 :test-result: FAIL EndsWith string matcher
 :test-result: PASS Enums can quickly have stringification enabled using REGISTER_ENUM
 :test-result: PASS Enums in namespaces can quickly have stringification enabled using REGISTER_ENUM

tests/SelfTest/Baselines/automake.sw.multi.approved.txt

@@ -129,7 +129,6 @@
 :test-result: PASS Default scale is invisible to comparison
 :test-result: PASS Directly creating an EnumInfo
 :test-result: PASS Empty stream name opens cout stream
-:test-result: PASS Empty tag is not allowed
 :test-result: FAIL EndsWith string matcher
 :test-result: PASS Enums can quickly have stringification enabled using REGISTER_ENUM
 :test-result: PASS Enums in namespaces can quickly have stringification enabled using REGISTER_ENUM

tests/SelfTest/Baselines/compact.sw.approved.txt

@@ -521,7 +521,6 @@ ToString.tests.cpp:<line number>: passed: enumInfo->lookup(3) == "{** unexpected
 ==
 "{** unexpected enum value **}"
 Stream.tests.cpp:<line number>: passed: Catch::makeStream( "" )->isConsole() for: true
-Tag.tests.cpp:<line number>: passed: Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
 Matchers.tests.cpp:<line number>: failed: testStringForMatching(), EndsWith( "Substring" ) for: "this string contains 'abc' as a substring" ends with: "Substring"
 Matchers.tests.cpp:<line number>: failed: testStringForMatching(), EndsWith( "this", Catch::CaseSensitive::No ) for: "this string contains 'abc' as a substring" ends with: "this" (case insensitive)
 EnumToString.tests.cpp:<line number>: passed: stringify( EnumClass3::Value1 ) == "Value1" for: "Value1" == "Value1"
@@ -2538,7 +2537,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
 InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
 Misc.tests.cpp:<line number>: passed:
 Misc.tests.cpp:<line number>: passed:
-test cases:  409 |  309 passed |  84 failed | 5 skipped | 11 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
-assertions: 2226 | 2049 passed | 145 failed | 32 failed as expected
+assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
 
 

tests/SelfTest/Baselines/compact.sw.multi.approved.txt

@@ -519,7 +519,6 @@ ToString.tests.cpp:<line number>: passed: enumInfo->lookup(3) == "{** unexpected
 ==
 "{** unexpected enum value **}"
 Stream.tests.cpp:<line number>: passed: Catch::makeStream( "" )->isConsole() for: true
-Tag.tests.cpp:<line number>: passed: Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
 Matchers.tests.cpp:<line number>: failed: testStringForMatching(), EndsWith( "Substring" ) for: "this string contains 'abc' as a substring" ends with: "Substring"
 Matchers.tests.cpp:<line number>: failed: testStringForMatching(), EndsWith( "this", Catch::CaseSensitive::No ) for: "this string contains 'abc' as a substring" ends with: "this" (case insensitive)
 EnumToString.tests.cpp:<line number>: passed: stringify( EnumClass3::Value1 ) == "Value1" for: "Value1" == "Value1"
@@ -2527,7 +2526,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
 InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
 Misc.tests.cpp:<line number>: passed:
 Misc.tests.cpp:<line number>: passed:
-test cases:  409 |  309 passed |  84 failed | 5 skipped | 11 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
-assertions: 2226 | 2049 passed | 145 failed | 32 failed as expected
+assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
 
 

tests/SelfTest/Baselines/console.std.approved.txt

@@ -1533,6 +1533,6 @@ due to unexpected exception with message:
   Why would you throw a std::string?
 
 ===============================================================================
-test cases:  409 |  323 passed |  69 failed | 6 skipped | 11 failed as expected
+test cases:  408 |  322 passed |  69 failed | 6 skipped | 11 failed as expected
-assertions: 2209 | 2049 passed | 128 failed | 32 failed as expected
+assertions: 2208 | 2048 passed | 128 failed | 32 failed as expected
 

tests/SelfTest/Baselines/console.sw.approved.txt

@@ -3967,15 +3967,6 @@ Stream.tests.cpp:<line number>: PASSED:
 with expansion:
   true
 
--------------------------------------------------------------------------------
-Empty tag is not allowed
--------------------------------------------------------------------------------
-Tag.tests.cpp:<line number>
-...............................................................................
-
-Tag.tests.cpp:<line number>: PASSED:
-  REQUIRE_THROWS( Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo ) )
-
 -------------------------------------------------------------------------------
 EndsWith string matcher
 -------------------------------------------------------------------------------
@@ -18231,6 +18222,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  409 |  309 passed |  84 failed | 5 skipped | 11 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
-assertions: 2226 | 2049 passed | 145 failed | 32 failed as expected
+assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
 

tests/SelfTest/Baselines/console.sw.multi.approved.txt

@@ -3965,15 +3965,6 @@ Stream.tests.cpp:<line number>: PASSED:
 with expansion:
   true
 
--------------------------------------------------------------------------------
-Empty tag is not allowed
--------------------------------------------------------------------------------
-Tag.tests.cpp:<line number>
-...............................................................................
-
-Tag.tests.cpp:<line number>: PASSED:
-  REQUIRE_THROWS( Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo ) )
-
 -------------------------------------------------------------------------------
 EndsWith string matcher
 -------------------------------------------------------------------------------
@@ -18220,6 +18211,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  409 |  309 passed |  84 failed | 5 skipped | 11 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
-assertions: 2226 | 2049 passed | 145 failed | 32 failed as expected
+assertions: 2225 | 2048 passed | 145 failed | 32 failed as expected
 

tests/SelfTest/Baselines/junit.sw.approved.txt

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="17" failures="128" skipped="11" tests="2237" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="128" skipped="11" tests="2236" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -463,7 +463,6 @@ at Exception.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Default scale is invisible to comparison" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Directly creating an EnumInfo" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Empty stream name opens cout stream" time="{duration}" status="run"/>
-    <testcase classname="<exe-name>.global" name="Empty tag is not allowed" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="EndsWith string matcher" time="{duration}" status="run">
       <failure message="testStringForMatching(), EndsWith( &quot;Substring&quot; )" type="CHECK_THAT">
 FAILED:

tests/SelfTest/Baselines/junit.sw.multi.approved.txt

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="<exe-name>" errors="17" failures="128" skipped="11" tests="2237" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="128" skipped="11" tests="2236" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -462,7 +462,6 @@ at Exception.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Default scale is invisible to comparison" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Directly creating an EnumInfo" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Empty stream name opens cout stream" time="{duration}" status="run"/>
-    <testcase classname="<exe-name>.global" name="Empty tag is not allowed" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="EndsWith string matcher" time="{duration}" status="run">
       <failure message="testStringForMatching(), EndsWith( &quot;Substring&quot; )" type="CHECK_THAT">
 FAILED:

tests/SelfTest/Baselines/sonarqube.sw.approved.txt

@@ -233,7 +233,6 @@
     <testCase name="startsWith" duration="{duration}"/>
   </file>
   <file path="tests/<exe-name>/IntrospectiveTests/Tag.tests.cpp">
-    <testCase name="Empty tag is not allowed" duration="{duration}"/>
     <testCase name="Tag alias can be registered against tag patterns/The same tag alias can only be registered once" duration="{duration}"/>
     <testCase name="Tag alias can be registered against tag patterns/Tag aliases must be of the form [@name]" duration="{duration}"/>
     <testCase name="Tags with spaces and non-alphanumerical characters are accepted" duration="{duration}"/>

tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt

@@ -232,7 +232,6 @@
     <testCase name="startsWith" duration="{duration}"/>
   </file>
   <file path="tests/<exe-name>/IntrospectiveTests/Tag.tests.cpp">
-    <testCase name="Empty tag is not allowed" duration="{duration}"/>
     <testCase name="Tag alias can be registered against tag patterns/The same tag alias can only be registered once" duration="{duration}"/>
     <testCase name="Tag alias can be registered against tag patterns/Tag aliases must be of the form [@name]" duration="{duration}"/>
     <testCase name="Tags with spaces and non-alphanumerical characters are accepted" duration="{duration}"/>

tests/SelfTest/Baselines/tap.sw.approved.txt

@@ -986,8 +986,6 @@ ok {test-number} - enumInfo->lookup(1) == "Value2" for: Value2 == "Value2"
 ok {test-number} - enumInfo->lookup(3) == "{** unexpected enum value **}" for: {** unexpected enum value **} == "{** unexpected enum value **}"
 # Empty stream name opens cout stream
 ok {test-number} - Catch::makeStream( "" )->isConsole() for: true
-# Empty tag is not allowed
-ok {test-number} - Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
 # EndsWith string matcher
 not ok {test-number} - testStringForMatching(), EndsWith( "Substring" ) for: "this string contains 'abc' as a substring" ends with: "Substring"
 # EndsWith string matcher
@@ -4477,5 +4475,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2237
+1..2236
 

tests/SelfTest/Baselines/tap.sw.multi.approved.txt

@@ -984,8 +984,6 @@ ok {test-number} - enumInfo->lookup(1) == "Value2" for: Value2 == "Value2"
 ok {test-number} - enumInfo->lookup(3) == "{** unexpected enum value **}" for: {** unexpected enum value **} == "{** unexpected enum value **}"
 # Empty stream name opens cout stream
 ok {test-number} - Catch::makeStream( "" )->isConsole() for: true
-# Empty tag is not allowed
-ok {test-number} - Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
 # EndsWith string matcher
 not ok {test-number} - testStringForMatching(), EndsWith( "Substring" ) for: "this string contains 'abc' as a substring" ends with: "Substring"
 # EndsWith string matcher
@@ -4466,5 +4464,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2237
+1..2236
 

tests/SelfTest/Baselines/teamcity.sw.approved.txt

@@ -301,8 +301,6 @@
 ##teamcity[testFinished name='Directly creating an EnumInfo' duration="{duration}"]
 ##teamcity[testStarted name='Empty stream name opens cout stream']
 ##teamcity[testFinished name='Empty stream name opens cout stream' duration="{duration}"]
-##teamcity[testStarted name='Empty tag is not allowed']
-##teamcity[testFinished name='Empty tag is not allowed' duration="{duration}"]
 ##teamcity[testStarted name='EndsWith string matcher']
 ##teamcity[testFailed name='EndsWith string matcher' message='Matchers.tests.cpp:<line number>|n...............................................................................|n|nMatchers.tests.cpp:<line number>|nexpression failed|n  CHECK_THAT( testStringForMatching(), EndsWith( "Substring" ) )|nwith expansion:|n  "this string contains |'abc|' as a substring" ends with: "Substring"|n']
 ##teamcity[testFailed name='EndsWith string matcher' message='Matchers.tests.cpp:<line number>|nexpression failed|n  CHECK_THAT( testStringForMatching(), EndsWith( "this", Catch::CaseSensitive::No ) )|nwith expansion:|n  "this string contains |'abc|' as a substring" ends with: "this" (case insensitive)|n']

tests/SelfTest/Baselines/teamcity.sw.multi.approved.txt

@@ -301,8 +301,6 @@
 ##teamcity[testFinished name='Directly creating an EnumInfo' duration="{duration}"]
 ##teamcity[testStarted name='Empty stream name opens cout stream']
 ##teamcity[testFinished name='Empty stream name opens cout stream' duration="{duration}"]
-##teamcity[testStarted name='Empty tag is not allowed']
-##teamcity[testFinished name='Empty tag is not allowed' duration="{duration}"]
 ##teamcity[testStarted name='EndsWith string matcher']
 ##teamcity[testFailed name='EndsWith string matcher' message='Matchers.tests.cpp:<line number>|n...............................................................................|n|nMatchers.tests.cpp:<line number>|nexpression failed|n  CHECK_THAT( testStringForMatching(), EndsWith( "Substring" ) )|nwith expansion:|n  "this string contains |'abc|' as a substring" ends with: "Substring"|n']
 ##teamcity[testFailed name='EndsWith string matcher' message='Matchers.tests.cpp:<line number>|nexpression failed|n  CHECK_THAT( testStringForMatching(), EndsWith( "this", Catch::CaseSensitive::No ) )|nwith expansion:|n  "this string contains |'abc|' as a substring" ends with: "this" (case insensitive)|n']

tests/SelfTest/Baselines/xml.sw.approved.txt

@@ -4375,17 +4375,6 @@ C
     </Expression>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <TestCase name="Empty tag is not allowed" filename="tests/<exe-name>/IntrospectiveTests/Tag.tests.cpp" >
-    <Expression success="true" type="REQUIRE_THROWS" filename="tests/<exe-name>/IntrospectiveTests/Tag.tests.cpp" >
-      <Original>
-        Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
-      </Original>
-      <Expanded>
-        Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
-      </Expanded>
-    </Expression>
-    <OverallResult success="true" skips="0"/>
-  </TestCase>
   <TestCase name="EndsWith string matcher" tags="[.][failing][matchers]" filename="tests/<exe-name>/UsageTests/Matchers.tests.cpp" >
     <Expression success="false" type="CHECK_THAT" filename="tests/<exe-name>/UsageTests/Matchers.tests.cpp" >
       <Original>
@@ -21203,6 +21192,6 @@ b1!
     </Section>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <OverallResults successes="2049" failures="145" expectedFailures="32" skips="11"/>
+  <OverallResults successes="2048" failures="145" expectedFailures="32" skips="11"/>
-  <OverallResultsCases successes="309" failures="84" expectedFailures="11" skips="5"/>
+  <OverallResultsCases successes="308" failures="84" expectedFailures="11" skips="5"/>
 </Catch2TestRun>

tests/SelfTest/Baselines/xml.sw.multi.approved.txt

@@ -4375,17 +4375,6 @@ C
     </Expression>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <TestCase name="Empty tag is not allowed" filename="tests/<exe-name>/IntrospectiveTests/Tag.tests.cpp" >
-    <Expression success="true" type="REQUIRE_THROWS" filename="tests/<exe-name>/IntrospectiveTests/Tag.tests.cpp" >
-      <Original>
-        Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
-      </Original>
-      <Expanded>
-        Catch::TestCaseInfo( "", { "fake test name", "[]" }, dummySourceLineInfo )
-      </Expanded>
-    </Expression>
-    <OverallResult success="true" skips="0"/>
-  </TestCase>
   <TestCase name="EndsWith string matcher" tags="[.][failing][matchers]" filename="tests/<exe-name>/UsageTests/Matchers.tests.cpp" >
     <Expression success="false" type="CHECK_THAT" filename="tests/<exe-name>/UsageTests/Matchers.tests.cpp" >
       <Original>
@@ -21202,6 +21191,6 @@ b1!
     </Section>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <OverallResults successes="2049" failures="145" expectedFailures="32" skips="11"/>
+  <OverallResults successes="2048" failures="145" expectedFailures="32" skips="11"/>
-  <OverallResultsCases successes="309" failures="84" expectedFailures="11" skips="5"/>
+  <OverallResultsCases successes="308" failures="84" expectedFailures="11" skips="5"/>
 </Catch2TestRun>

tests/SelfTest/IntrospectiveTests/Tag.tests.cpp

@@ -98,7 +98,20 @@ TEST_CASE( "Test case with identical tags keeps just one", "[tags]" ) {
     REQUIRE( testCase.tags[0] == Tag( "tag1" ) );
 }
 
-TEST_CASE( "Empty tag is not allowed" ) {
+TEST_CASE("Mismatched square brackets in tags are caught and reported",
-    REQUIRE_THROWS( Catch::TestCaseInfo(
+          "[tags][approvals]") {
-        "", { "fake test name", "[]" }, dummySourceLineInfo ) );
+    using Catch::TestCaseInfo;
+    using Catch::Matchers::ContainsSubstring;
+            REQUIRE_THROWS_WITH( TestCaseInfo( "",
+                                       { "test with unclosed tag", "[abc" },
+                                       dummySourceLineInfo ),
+                         ContainsSubstring("registering test case 'test with unclosed tag'") );
+    REQUIRE_THROWS_WITH( TestCaseInfo( "",
+                      { "test with nested tags", "[abc[def]]" },
+                      dummySourceLineInfo ),
+        ContainsSubstring("registering test case 'test with nested tags'") );
+    REQUIRE_THROWS_WITH( TestCaseInfo( "",
+                      { "test with superfluous close tags", "[abc][def]]" },
+                      dummySourceLineInfo ),
+        ContainsSubstring("registering test case 'test with superfluous close tags'") );
 }