Make --list-* exit code be 0

Previously it returned the sum of listed things because ???. This
was completely useless and in many ways actively counterproductive
because of the success/failure conventions around exit codes.

Closes #1410

docs/release-notes.md

@@ -37,6 +37,8 @@
 
 ### Fixes
 * The `INFO` macro no longer contains superfluous semicolon (#1456)
+* The `--list*` family of command line flags now return 0 on success (#1410, #1146)
+
 
 ## 2.10.2
 

include/internal/catch_list.cpp

@@ -24,10 +24,21 @@
 #include <limits>
 #include <algorithm>
 #include <iomanip>
+#include <set>
 
 namespace Catch {
+    namespace {
 
-    std::size_t listTests( Config const& config ) {
+        struct TagInfo {
+            void add(std::string const& spelling);
+            std::string all() const;
+
+            std::set<std::string> spellings;
+            std::size_t count = 0;
+        };
+
+
+        void listTests(Config const& config) {
             TestSpec testSpec = config.testSpec();
             if (config.hasTestFilters())
                 Catch::cout() << "Matching test cases:\n";
@@ -58,10 +69,9 @@ namespace Catch {
                 Catch::cout() << pluralise(matchedTestCases.size(), "test case") << '\n' << std::endl;
             else
                 Catch::cout() << pluralise(matchedTestCases.size(), "matching test case") << '\n' << std::endl;
-        return matchedTestCases.size();
         }
 
-    std::size_t listTestsNamesOnly( Config const& config ) {
+        void listTestsNamesOnly(Config const& config) {
             TestSpec testSpec = config.testSpec();
             std::size_t matchedTests = 0;
             std::vector<TestCase> matchedTestCases = filterTests(getAllTestCasesSorted(config), testSpec, config);
@@ -75,31 +85,9 @@ namespace Catch {
                     Catch::cout() << "\t@" << testCaseInfo.lineInfo;
                 Catch::cout() << std::endl;
             }
-        return matchedTests;
         }
 
-    void TagInfo::add( std::string const& spelling ) {
+        void listTags(Config const& config) {
-        ++count;
-        spellings.insert( spelling );
-    }
-
-    std::string TagInfo::all() const {
-        size_t size = 0;
-        for (auto const& spelling : spellings) {
-            // Add 2 for the brackes
-            size += spelling.size() + 2;
-        }
-
-        std::string out; out.reserve(size);
-        for (auto const& spelling : spellings) {
-            out += '[';
-            out += spelling;
-            out += ']';
-        }
-        return out;
-    }
-
-    std::size_t listTags( Config const& config ) {
             TestSpec testSpec = config.testSpec();
             if (config.hasTestFilters())
                 Catch::cout() << "Tags for matching test cases:\n";
@@ -131,10 +119,9 @@ namespace Catch {
                 Catch::cout() << str << wrapper << '\n';
             }
             Catch::cout() << pluralise(tagCounts.size(), "tag") << '\n' << std::endl;
-        return tagCounts.size();
         }
 
-    std::size_t listReporters() {
+        void listReporters() {
             Catch::cout() << "Available reporters:\n";
             IReporterRegistry::FactoryMap const& factories = getRegistryHub().getReporterRegistry().getFactories();
             std::size_t maxNameLen = 0;
@@ -153,21 +140,51 @@ namespace Catch {
                     << "\n";
             }
             Catch::cout() << std::endl;
-        return factories.size();
         }
 
-    Option<std::size_t> list( std::shared_ptr<Config> const& config ) {
+    } // end anonymous namespace
-        Option<std::size_t> listedCount;
+
+    void TagInfo::add( std::string const& spelling ) {
+        ++count;
+        spellings.insert( spelling );
+    }
+
+    std::string TagInfo::all() const {
+        size_t size = 0;
+        for (auto const& spelling : spellings) {
+            // Add 2 for the brackes
+            size += spelling.size() + 2;
+        }
+
+        std::string out; out.reserve(size);
+        for (auto const& spelling : spellings) {
+            out += '[';
+            out += spelling;
+            out += ']';
+        }
+        return out;
+    }
+
+    bool list( std::shared_ptr<Config> const& config ) {
+        bool listed = false;
         getCurrentMutableContext().setConfig( config );
-        if( config->listTests() )
+        if (config->listTests()) {
-            listedCount = listedCount.valueOr(0) + listTests( *config );
+            listed = true;
-        if( config->listTestNamesOnly() )
+            listTests(*config);
-            listedCount = listedCount.valueOr(0) + listTestsNamesOnly( *config );
+        }
-        if( config->listTags() )
+        if (config->listTestNamesOnly()) {
-            listedCount = listedCount.valueOr(0) + listTags( *config );
+            listed = true;
-        if( config->listReporters() )
+            listTestsNamesOnly(*config);
-            listedCount = listedCount.valueOr(0) + listReporters();
+        }
-        return listedCount;
+        if (config->listTags()) {
+            listed = true;
+            listTags(*config);
+        }
+        if (config->listReporters()) {
+            listed = true;
+            listReporters();
+        }
+        return listed;
     }
 
 } // end namespace Catch

include/internal/catch_list.h

@@ -8,30 +8,11 @@
 #ifndef TWOBLUECUBES_CATCH_LIST_H_INCLUDED
 #define TWOBLUECUBES_CATCH_LIST_H_INCLUDED
 
-#include "catch_option.hpp"
 #include "catch_config.hpp"
 
-#include <set>
-
 namespace Catch {
 
-    std::size_t listTests( Config const& config );
+    bool list( std::shared_ptr<Config> const& config );
-
-    std::size_t listTestsNamesOnly( Config const& config );
-
-    struct TagInfo {
-        void add( std::string const& spelling );
-        std::string all() const;
-
-        std::set<std::string> spellings;
-        std::size_t count = 0;
-    };
-
-    std::size_t listTags( Config const& config );
-
-    std::size_t listReporters();
-    
-    Option<std::size_t> list( std::shared_ptr<Config> const& config );
 
 } // end namespace Catch
 

include/internal/catch_session.cpp

@@ -288,8 +288,9 @@ namespace Catch {
                 applyFilenamesAsTags( *m_config );
 
             // Handle list request
-            if( Option<std::size_t> listed = list( m_config ) )
+            if (list(m_config)) {
-                return static_cast<int>( *listed );
+                return 0;
+            }
 
             TestGroup tests { m_config };
             auto const totals = tests.execute();

projects/CMakeLists.txt

@@ -376,24 +376,33 @@ set_tests_properties(RunTests PROPERTIES
     FAIL_REGULAR_EXPRESSION "Filters:"
 )
 
-add_test(NAME ListTests COMMAND $<TARGET_FILE:SelfTest> --list-tests --verbosity high)
+# Because CTest does not allow us to check both return code _and_ expected
-set_tests_properties(ListTests PROPERTIES 
+# output in one test, we run these commands twice. First time we check
+# the output, the second time we check the exit code.
+add_test(NAME List::Tests::Output COMMAND $<TARGET_FILE:SelfTest> --list-tests --verbosity high)
+set_tests_properties(List::Tests::Output PROPERTIES
     PASS_REGULAR_EXPRESSION "[0-9]+ test cases"
     FAIL_REGULAR_EXPRESSION "Hidden Test"
 )
+add_test(NAME List::Tests::ExitCode COMMAND $<TARGET_FILE:SelfTest> --list-tests --verbosity high)
 
-add_test(NAME ListTags COMMAND $<TARGET_FILE:SelfTest> --list-tags)
+add_test(NAME List::Tags::Output COMMAND $<TARGET_FILE:SelfTest> --list-tags)
-set_tests_properties(ListTags PROPERTIES 
+set_tests_properties(List::Tags::Output PROPERTIES
     PASS_REGULAR_EXPRESSION "[0-9]+ tags"
     FAIL_REGULAR_EXPRESSION "\\[\\.\\]")
+add_test(NAME List::Tags::ExitCode COMMAND $<TARGET_FILE:SelfTest> --list-tags)
 
-add_test(NAME ListReporters COMMAND $<TARGET_FILE:SelfTest> --list-reporters)
+add_test(NAME List::Reporters::Output COMMAND $<TARGET_FILE:SelfTest> --list-reporters)
-set_tests_properties(ListReporters PROPERTIES PASS_REGULAR_EXPRESSION "Available reporters:")
+set_tests_properties(List::Reporters::Output PROPERTIES PASS_REGULAR_EXPRESSION "Available reporters:")
+add_test(NAME List::Reporters::ExitCode COMMAND $<TARGET_FILE:SelfTest> --list-reporters)
 
-add_test(NAME ListTestNamesOnly COMMAND $<TARGET_FILE:SelfTest> --list-test-names-only)
+add_test(NAME List::Tests::NamesOnly::Output COMMAND $<TARGET_FILE:SelfTest> --list-test-names-only)
-set_tests_properties(ListTestNamesOnly PROPERTIES 
+set_tests_properties(List::Tests::NamesOnly::Output PROPERTIES
     PASS_REGULAR_EXPRESSION "Regex string matcher"
     FAIL_REGULAR_EXPRESSION "Hidden Test")
+add_test(NAME List::Tests::NamesOnly::ExitCode COMMAND $<TARGET_FILE:SelfTest> --list-test-names-only)
+
+
 
 add_test(NAME NoAssertions COMMAND $<TARGET_FILE:SelfTest> -w NoAssertions)
 set_tests_properties(NoAssertions PROPERTIES PASS_REGULAR_EXPRESSION "No assertions in test case")