kyua: Do not count skipped as passed in test cmd

It changes output of 'kyua test' CLI command only. Hence, other outputs
like junit are kept intact for CI and other use cases. It's meant to
improve UX of attended use cases.

The issue is that the following can be tricky to interpret:

  222/222 passed (0 failed)

It can be read as all tests are passed, but it might be a summary line
of all tests skipped due to some requirement is not met.

It's reworked to easily distinguish such cases:

  222/222 passed (0 broken, 0 failed, 0 skipped)
  0/222 passed (0 broken, 0 failed, 222 skipped)

The overall formula is:

  <actually passed>/<total> (<details about not actually passed ones>)

Suggested by:	kp
Reviewed by:	ngie, markj
Approved by:	markj (mentor)
Differential Revision:	https://reviews.freebsd.org/D46653

contrib/kyua/cli/cmd_test.cpp

@@ -64,11 +64,8 @@ class print_hooks : public drivers::run_tests::base_hooks {
     bool _parallel;
 
 public:
-    /// The amount of positive test results found so far.
-    unsigned long good_count;
-
-    /// The amount of negative test results found so far.
-    unsigned long bad_count;
+    /// The amount of test results per type.
+    std::map<enum model::test_result_type, unsigned long> type_count;
 
     /// Constructor for the hooks.
     ///
@@ -76,10 +73,10 @@ public:
     /// \param parallel_ True if we are executing more than one test at once.
     print_hooks(cmdline::ui* ui_, const bool parallel_) :
         _ui(ui_),
-        _parallel(parallel_),
-        good_count(0),
-        bad_count(0)
+        _parallel(parallel_)
     {
+        for (const auto& pair : model::test_result_types)
+            type_count[pair.first] = 0;
     }
 
     /// Called when the processing of a test case begins.
@@ -116,10 +113,8 @@ public:
         }
         _ui->out(F("%s  [%s]") % cli::format_result(result) %
             cli::format_delta(duration));
-        if (result.good())
-            good_count++;
-        else
-            bad_count++;
+
+        type_count[result.type()]++;
     }
 };
 
@@ -159,8 +154,21 @@ cmd_test::run(cmdline::ui* ui, const cmdline::parsed_cmdline& cmdline,
         kyuafile_path(cmdline), build_root_path(cmdline), results.second,
         parse_filters(cmdline.arguments()), user_config, hooks);
 
+    unsigned long total = 0;
+    unsigned long good = 0;
+    unsigned long bad = 0;
+    for (const auto& pair : model::test_result_types) {
+        const auto& type = pair.second;
+        const auto count = hooks.type_count[type.id];
+        total += count;
+        if (type.is_run && type.is_good)
+            good += count;
+        if (!type.is_good)
+            bad += count;
+    }
+
     int exit_code;
-    if (hooks.good_count > 0 || hooks.bad_count > 0) {
+    if (total > 0) {
         ui->out("");
         if (!results.first.empty()) {
             ui->out(F("Results file id is %s") % results.first);
@@ -168,10 +176,20 @@ cmd_test::run(cmdline::ui* ui, const cmdline::parsed_cmdline& cmdline,
         ui->out(F("Results saved to %s") % results.second);
         ui->out("");
 
-        ui->out(F("%s/%s passed (%s failed)") % hooks.good_count %
-                (hooks.good_count + hooks.bad_count) % hooks.bad_count);
+        ui->out(F("%s/%s passed (") % good % total, false);
+        const auto& types = model::test_result_types;
+        for (auto it = types.begin(); it != types.end(); it++) {
+            const auto& type = it->second;
+            if (!type.is_run || !type.is_good) {
+                if (it != types.begin())
+                    ui->out(", ", false);
+                ui->out(F("%s %s") % hooks.type_count[type.id] % type.name,
+                    false);
+            }
+        }
+        ui->out(")");
 
-        exit_code = (hooks.bad_count == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+        exit_code = (bad == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
     } else {
         // TODO(jmmv): Delete created empty file; it's useless!
         if (!results.first.empty()) {

contrib/kyua/integration/cmd_test_test.sh

@@ -44,7 +44,7 @@ simple_all_pass:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/2 passed (0 failed)
+1/2 passed (0 broken, 0 failed, 1 skipped)
 EOF
 
     utils_cp_helper simple_all_pass .
@@ -69,7 +69,7 @@ simple_some_fail:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-1/2 passed (1 failed)
+1/2 passed (0 broken, 1 failed, 0 skipped)
 EOF
 
     utils_cp_helper simple_some_fail .
@@ -102,7 +102,7 @@ third:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-7/7 passed (0 failed)
+3/7 passed (0 broken, 0 failed, 4 skipped)
 EOF
 
     utils_cp_helper simple_all_pass first
@@ -138,7 +138,7 @@ third:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-4/7 passed (3 failed)
+3/7 passed (0 broken, 3 failed, 1 skipped)
 EOF
 
     utils_cp_helper simple_some_fail first
@@ -172,7 +172,7 @@ expect_all_pass:timeout  ->  expected_failure: This times out  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-5/5 passed (0 failed)
+5/5 passed (0 broken, 0 failed, 0 skipped)
 EOF
 # CHECK_STYLE_ENABLE
 
@@ -203,7 +203,7 @@ expect_some_fail:timeout  ->  failed: Test case was expected to hang but it cont
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-1/6 passed (5 failed)
+1/6 passed (0 broken, 5 failed, 0 skipped)
 EOF
 # CHECK_STYLE_ENABLE
 
@@ -231,7 +231,7 @@ bogus_test_cases:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-1/3 passed (2 failed)
+1/3 passed (2 broken, 0 failed, 0 skipped)
 EOF
 # CHECK_STYLE_ENABLE
 
@@ -270,7 +270,7 @@ subdir/simple_some_fail:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-3/4 passed (1 failed)
+2/4 passed (0 broken, 1 failed, 1 skipped)
 EOF
     atf_check -s exit:1 -o file:expout -e empty kyua test
 }
@@ -302,7 +302,7 @@ subdir/simple_all_pass:skip  ->  skipped: The reason for skipping is this  [S.UU
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/2 passed (0 failed)
+1/2 passed (0 broken, 0 failed, 1 skipped)
 EOF
 # CHECK_STYLE_ENABLE
     atf_check -s exit:0 -o file:expout -e empty kyua test subdir
@@ -328,7 +328,7 @@ first:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-1/1 passed (0 failed)
+0/1 passed (0 broken, 0 failed, 1 skipped)
 EOF
     atf_check -s exit:0 -o file:expout -e empty kyua test first:skip
 }
@@ -354,7 +354,7 @@ second:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-1/2 passed (1 failed)
+1/2 passed (0 broken, 1 failed, 0 skipped)
 EOF
     atf_check -s exit:1 -o file:expout -e empty kyua test second
 }
@@ -402,7 +402,7 @@ subdir/second:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/3 passed (1 failed)
+2/3 passed (0 broken, 1 failed, 0 skipped)
 EOF
     atf_check -s exit:1 -o file:expout -e empty kyua test subdir first:pass
 }
@@ -470,7 +470,7 @@ third:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-3/4 passed (1 failed)
+2/4 passed (0 broken, 1 failed, 1 skipped)
 EOF
 
     cat >experr <<EOF
@@ -515,7 +515,7 @@ subdir/fourth:fail  ->  failed: This fails on purpose  [S.UUUs]
 Results file id is $(utils_results_id root)
 Results saved to $(utils_results_file root)
 
-2/3 passed (1 failed)
+1/3 passed (0 broken, 1 failed, 1 skipped)
 EOF
     atf_check -s exit:1 -o file:expout -e empty kyua test \
         -k "$(pwd)/root/Kyuafile" first subdir/fourth:fail
@@ -542,7 +542,7 @@ first:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/2 passed (0 failed)
+1/2 passed (0 broken, 0 failed, 1 skipped)
 EOF
     CREATE_COOKIE="$(pwd)/cookie"; export CREATE_COOKIE
     atf_check -s exit:0 -o file:expout -e empty kyua test first
@@ -612,7 +612,7 @@ some-program:pass  ->  passed  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-1/2 passed (1 failed)
+1/2 passed (0 broken, 1 failed, 0 skipped)
 EOF
 
     atf_check -s exit:1 -o file:expout -e empty kyua test
@@ -710,7 +710,7 @@ subdir/third:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-6/6 passed (0 failed)
+3/6 passed (0 broken, 0 failed, 3 skipped)
 EOF
 
     mkdir build
@@ -745,7 +745,7 @@ sometest:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/2 passed (0 failed)
+1/2 passed (0 broken, 0 failed, 1 skipped)
 EOF
     atf_check -s exit:0 -o file:expout -e empty kyua test -k myfile
     atf_check -s exit:0 -o file:expout -e empty kyua test --kyuafile=myfile
@@ -774,7 +774,7 @@ sometest:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/2 passed (0 failed)
+1/2 passed (0 broken, 0 failed, 1 skipped)
 EOF
     atf_check -s exit:0 -o file:expout -e empty kyua test -k myfile sometest
     cat >expout <<EOF
@@ -784,7 +784,7 @@ sometest:skip  ->  skipped: The reason for skipping is this  [S.UUUs]
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-2/2 passed (0 failed)
+1/2 passed (0 broken, 0 failed, 1 skipped)
 EOF
     atf_check -s exit:0 -o file:expout -e empty kyua test --kyuafile=myfile \
         sometest
@@ -987,7 +987,7 @@ non_executable:__test_cases_list__  ->  broken: Permission denied to run test pr
 Results file id is $(utils_results_id)
 Results saved to $(utils_results_file)
 
-0/2 passed (2 failed)
+0/2 passed (2 broken, 0 failed, 0 skipped)
 EOF
 # CHECK_STYLE_ENABLE
     atf_check -s exit:1 -o file:expout -e empty kyua test

contrib/kyua/model/test_result.cpp

@@ -35,6 +35,42 @@
 namespace text = utils::text;
 
 
+const std::map<enum model::test_result_type,
+               const struct model::test_result_type_desc>
+    model::test_result_types =
+{
+    { test_result_broken,
+        { .id =      test_result_broken,
+          .name =    "broken",
+          .is_run =  true,
+          .is_good = false, } },
+
+    { test_result_expected_failure,
+        { .id =      test_result_expected_failure,
+          .name =    "xfail",
+          .is_run =  true,
+          .is_good = true, } },
+
+    { test_result_failed,
+        { .id =      test_result_failed,
+          .name =    "failed",
+          .is_run =  true,
+          .is_good = false, } },
+
+    { test_result_passed,
+        { .id =      test_result_passed,
+          .name =    "passed",
+          .is_run =  true,
+          .is_good = true, } },
+
+    { test_result_skipped,
+        { .id =      test_result_skipped,
+          .name =    "skipped",
+          .is_run =  false,
+          .is_good = true, } },
+};
+
+
 /// Constructs a base result.
 ///
 /// \param type_ The type of the result.
@@ -74,17 +110,7 @@ model::test_result::reason(void) const
 bool
 model::test_result::good(void) const
 {
-    switch (_type) {
-    case test_result_expected_failure:
-    case test_result_passed:
-    case test_result_skipped:
-        return true;
-
-    case test_result_broken:
-    case test_result_failed:
-        return false;
-    }
-    UNREACHABLE;
+    return test_result_types.at(_type).is_good;
 }
 
 

contrib/kyua/model/test_result.hpp

@@ -34,12 +34,27 @@
 
 #include "model/test_result_fwd.hpp"
 
+#include <map>
 #include <ostream>
 #include <string>
 
 namespace model {
 
 
+/// Test result type metadata.
+struct test_result_type_desc {
+    enum test_result_type id;
+    std::string name;
+    bool is_run;
+    bool is_good;
+};
+
+
+/// Description of each test result type.
+extern const std::map<enum test_result_type,
+    const struct test_result_type_desc> test_result_types;
+
+
 /// Representation of a single test result.
 ///
 /// A test result is a simple pair of (type, reason).  The type indicates the