stats: Stop reporting passing assertions.

Before this fix, assertions were always reported to the runner by the
test, even when passing, for accuracy w.r.t statistics.

This wasn't really a problem as prior to 2.3.x, criterion was using
a very simple protocol to pass its data, which could handle passing
extra data for correctness.

However, since the I/O layer rewrite that introduced a more complex protocol
format between the runner and the tests, passing repeatedly extra data
for passed assertions incurred a huge slowdown for large number of
assertions.

In this situation, and because data from passed assertions is rarely
used, we provide a mean to trade performance at the cost of
correctness: We introduce a `full_stats` mode, which when activated
reproduces the old behaviour, and a new behaviour in which tests stop
reporting passing assertions, and only send an update to the count of
passing assertions (without any detail attached).

Users implementing custom reporting mechanisms in which having full
access to the data of passing assertions will still be able to access it
by using the `--full-stats` CLI switch.

This fixes #172.

.uncrustify.cfg

@@ -150,10 +150,10 @@ sp_func_proto_paren             = remove        # "int foo ();" vs "int foo();"
 sp_getset_brace                 = force
 sp_incdec                       = remove
 sp_inside_angle                 = remove
-sp_inside_braces                = force         # "{ 1 }" vs "{1}"
+sp_inside_braces                = add         # "{ 1 }" vs "{1}"
 sp_inside_braces_empty          = remove
-sp_inside_braces_enum           = force         # "{ 1 }" vs "{1}"
-sp_inside_braces_struct         = force         # "{ 1 }" vs "{1}"
+sp_inside_braces_enum           = add         # "{ 1 }" vs "{1}"
+sp_inside_braces_struct         = add         # "{ 1 }" vs "{1}"
 sp_inside_fparen                = remove
 sp_inside_fparens               = remove
 sp_inside_paren                 = remove
@@ -202,7 +202,7 @@ align_on_tabstop                = false         # align on tabstops
 align_enum_equ_span             = 4             # '=' in enum definition
 align_nl_cont                   = true
 align_assign_span               = 0
-align_struct_init_span          = 3             # align stuff in a structure init '= { }'
+align_struct_init_span          = 0             # align stuff in a structure init '= { }'
 align_right_cmt_span            = 3
 align_pp_define_span            = 2
 align_pp_define_gap             = 4

doc/env.rst

@@ -37,6 +37,11 @@ Command line arguments
   equivalent to ``--output=tap:FILE``.
 * ``--verbose[=level]``: Makes the output verbose. When provided with an integer,
   sets the verbosity level to that integer.
+* ``--full-stats``: Forces tests to fully report statistics. By default,
+  tests do not report details for passing assertions, so this option forces
+  them to do so.
+  Activating this causes massive slowdowns for large number of assertions, but
+  provides more accurate reports.
 * ``-OPROVIDER:FILE or --output=PROVIDER:FILE``: Write a test report to FILE
   using the output provider named by PROVIDER.
   If FILE is ``"-"``, it implies ``--quiet``, and the report shall be written

include/criterion/internal/assert.h

@@ -44,6 +44,7 @@
 #include "../hooks.h"
 #include "../event.h"
 #include "../abort.h"
+#include "../options.h"
 
 struct criterion_assert_args {
     const char *msg;
@@ -75,6 +76,7 @@ enum criterion_assert_messages {
 CR_BEGIN_C_API
 
 CR_API char *cr_translate_assert_msg(int msg_index, ...);
+CR_API void cri_asserts_passed_incr(void);
 
 CR_END_C_API
 
@@ -110,25 +112,28 @@ CR_END_C_API
 #define CR_FAIL_ABORT_        criterion_abort_test
 #define CR_FAIL_CONTINUES_    criterion_continue_test
 
-#define cr_assert_impl(Fail, Condition, ...)             \
-    do {                                                 \
-        bool cr_passed__ = !!(Condition);                \
-        char *cr_msg__ = NULL;                           \
-        int cr_shifted__ = 0;                            \
-                                                         \
-        CR_EXPAND(CR_INIT_STATS_(cr_msg__, cr_shifted__, \
-                CR_VA_TAIL(__VA_ARGS__)));               \
-        struct criterion_assert_stats cr_stat__;         \
-        cr_stat__.passed = cr_passed__;                  \
-        cr_stat__.file = __FILE__;                       \
-        cr_stat__.line = __LINE__;                       \
-        cr_stat__.message = cr_msg__;                    \
-        criterion_send_assert(&cr_stat__);               \
-                                                         \
-        cr_asprintf_free(cr_msg__ - cr_shifted__);       \
-                                                         \
-        if (!cr_passed__)                                \
-            Fail();                                      \
+#define cr_assert_impl(Fail, Condition, ...)                 \
+    do {                                                     \
+        bool cr_passed__ = !!(Condition);                    \
+        char *cr_msg__ = NULL;                               \
+        int cr_shifted__ = 0;                                \
+                                                             \
+        if (!cr_passed__ || criterion_options.full_stats) {  \
+            CR_EXPAND(CR_INIT_STATS_(cr_msg__, cr_shifted__, \
+                    CR_VA_TAIL(__VA_ARGS__)));               \
+            struct criterion_assert_stats cr_stat__;         \
+            cr_stat__.passed = cr_passed__;                  \
+            cr_stat__.file = __FILE__;                       \
+            cr_stat__.line = __LINE__;                       \
+            cr_stat__.message = cr_msg__;                    \
+            criterion_send_assert(&cr_stat__);               \
+                                                             \
+            cr_asprintf_free(cr_msg__ - cr_shifted__);       \
+        }                                                    \
+        if (!cr_passed__)                                    \
+            Fail();                                          \
+        else                                                 \
+            cri_asserts_passed_incr();                       \
     } while (0)
 
 #define cr_fail(Fail, ...)                 \

include/criterion/internal/test.h

@@ -68,6 +68,7 @@ CR_BEGIN_C_API
 CR_API void criterion_internal_test_setup(void);
 CR_API void criterion_internal_test_main(void (*fn)(void));
 CR_API void criterion_internal_test_teardown(void);
+CR_API void cri_asserts_passed_incr(void);
 
 CR_END_C_API
 
@@ -169,8 +170,8 @@ static const char *const cr_msg_test_fini_other_exception = "Caught some unexpec
     struct criterion_test CR_IDENTIFIER_(Category, Name, meta) = {           \
         #Name,                                                               \
         #Category,                                                           \
-        CR_IDENTIFIER_(Category,  Name, jmp),                                \
-        &CR_IDENTIFIER_(Category, Name, extra)                               \
+        CR_IDENTIFIER_(Category,  Name,  jmp),                               \
+        &CR_IDENTIFIER_(Category, Name,  extra)                              \
     };                                                                       \
     CR_SECTION_("cr_tst")                                                    \
     struct criterion_test *CR_IDENTIFIER_(Category, Name, ptr)               \

include/criterion/options.h

@@ -180,6 +180,17 @@ struct criterion_options {
      *  default: 0
      */
     double timeout;
+
+    /**
+     * Fully report statistics from test workers, including those that are
+     * not reported by default (like passing assertions).
+     *
+     * Reporting everything leads to more accurate reports at the cost of
+     * execution speed.
+     *
+     * default: false
+     */
+    bool full_stats;
 };
 
 CR_BEGIN_C_API

src/atomic.h

@@ -0,0 +1,46 @@
+/*
+ * The MIT License (MIT)
+ *
+ * Copyright © 2015-2016 Franklin "Snaipe" Mathieu <http://snai.pe/>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef ATOMIC_H_
+#define ATOMIC_H_
+
+#include <stdint.h>
+
+#ifdef __GNUC__
+# define cri_atomic_add(Val, Add)    __sync_fetch_and_add(&(Val), (Add))
+#else
+# define cri_atomic_add(Val, Add)    cri_atomic_add_impl((uintptr_t *) &(Val), (Add))
+
+static inline uintptr_t cri_atomic_add_impl(uintptr_t *val, uintptr_t add)
+{
+# if defined (_WIN64)
+    return InterlockedExchangeAdd64(val, add);
+# elif defined (_WIN32)
+    return InterlockedExchangeAdd(val, add);
+# else
+#  error Atomics are not supported on your system
+# endif
+}
+#endif
+
+#endif /* !ATOMIC_H_ */

src/core/abort.c

@@ -32,6 +32,7 @@
 #include "debugbreak.h"
 
 jmp_buf g_pre_test;
+size_t cri_nb_asserts;
 
 void criterion_abort_test(void)
 {

src/core/client.c

@@ -66,13 +66,15 @@ bool handle_phase(struct server_ctx *, struct client_ctx *, const criterion_prot
 bool handle_death(struct server_ctx *, struct client_ctx *, const criterion_protocol_msg *);
 bool handle_assert(struct server_ctx *, struct client_ctx *, const criterion_protocol_msg *);
 bool handle_message(struct server_ctx *, struct client_ctx *, const criterion_protocol_msg *);
+bool handle_statistic(struct server_ctx *, struct client_ctx *, const criterion_protocol_msg *);
 
 static message_handler *message_handlers[] = {
-    [criterion_protocol_submessage_birth_tag]   = handle_birth,
-    [criterion_protocol_submessage_phase_tag]   = handle_phase,
-    [criterion_protocol_submessage_death_tag]   = handle_death,
-    [criterion_protocol_submessage_assert_tag]  = handle_assert,
-    [criterion_protocol_submessage_message_tag] = handle_message,
+    [criterion_protocol_submessage_birth_tag]       = handle_birth,
+    [criterion_protocol_submessage_phase_tag]       = handle_phase,
+    [criterion_protocol_submessage_death_tag]       = handle_death,
+    [criterion_protocol_submessage_assert_tag]      = handle_assert,
+    [criterion_protocol_submessage_message_tag]     = handle_message,
+    [criterion_protocol_submessage_statistic_tag]   = handle_statistic,
 };
 
 static void get_message_id(char *out, size_t n, const criterion_protocol_msg *msg)
@@ -522,6 +524,21 @@ bool handle_assert(struct server_ctx *sctx, struct client_ctx *ctx, const criter
     return false;
 }
 
+bool handle_statistic(struct server_ctx *sctx, struct client_ctx *ctx, const criterion_protocol_msg *msg)
+{
+    (void) sctx;
+    const criterion_protocol_statistic *stat = &msg->data.value.statistic;
+    /* TODO: handle this better with new statistics API */
+    if (!strcmp(stat->key, ".asserts_passed") && stat->which_value == criterion_protocol_statistic_num_tag) {
+        ctx->tstats->passed_asserts += stat->value.num;
+        ctx->sstats->asserts_passed += stat->value.num;
+        ctx->sstats->nb_asserts     += stat->value.num;
+        ctx->gstats->asserts_passed += stat->value.num;
+        ctx->gstats->nb_asserts     += stat->value.num;
+    }
+    return false;
+}
+
 bool handle_message(struct server_ctx *sctx, struct client_ctx *ctx, const criterion_protocol_msg *msg)
 {
     (void) sctx;

src/core/test.c

@@ -31,10 +31,18 @@
 #include "protocol/protocol.h"
 #include "protocol/messages.h"
 #include "io/event.h"
+#include "atomic.h"
 
 extern const struct criterion_test *criterion_current_test;
 extern const struct criterion_suite *criterion_current_suite;
 
+static size_t passed_asserts;
+
+void cri_asserts_passed_incr(void)
+{
+    cri_atomic_add(passed_asserts, 1);
+}
+
 static void send_event(int phase)
 {
     criterion_protocol_msg msg = criterion_message(phase,
@@ -86,6 +94,16 @@ void criterion_internal_test_main(void (*fn)(void))
         }
     }
 
+    if (!criterion_options.full_stats) {
+        criterion_protocol_msg msg = criterion_message_nots(statistic,
+                        .key = ".asserts_passed",
+                        .which_value = criterion_protocol_statistic_num_tag,
+                        .value.num = passed_asserts);
+
+        criterion_message_set_id(msg);
+        cr_send_to_runner(&msg);
+    }
+
     send_event(criterion_protocol_phase_kind_TEARDOWN);
 }
 

src/entry/params.c

@@ -76,6 +76,9 @@
     "default. TYPE may be gdb, lldb, or wingbd.\n"          \
     "    --debug-transport=VAL: the transport to use by "   \
     "the debugging server. `tcp:1234` by default\n"         \
+    "    --full-stats: Tests must fully report statistics " \
+    "(causes massive slowdown for large number of "         \
+    "assertions but is more accurate)."                     \
     "    -OP:F or --output=PROVIDER=FILE: write test "      \
     "report to FILE using the specified provider\n"
 
@@ -215,6 +218,7 @@ CR_API int criterion_handle_args(int argc, char *argv[],
         { "crash",           no_argument,       0, 'c' },
         { "debug",           optional_argument, 0, 'd' },
         { "debug-transport", required_argument, 0, 'D' },
+        { "full-stats",      no_argument,       0, 'U' },
         { 0,                 0,                 0, 0   }
     };
 
@@ -354,6 +358,7 @@ CR_API int criterion_handle_args(int argc, char *argv[],
                 fprintf(stderr, "--single has been removed. Use --debug instead.");
                 exit(3);
             case 'c': criterion_options.crash = true; break;
+            case 'U': criterion_options.full_stats = true; break;
             case '?':
             default: do_print_usage = handle_unknown_arg; break;
         }

src/protocol/criterion.options

@@ -6,3 +6,5 @@ criterion.protocol.phase.name       type : FT_POINTER
 criterion.protocol.phase.message    type : FT_POINTER
 criterion.protocol.ack.message      type : FT_POINTER
 criterion.protocol.msg.uid          type : FT_POINTER
+criterion.protocol.statistic.key    type : FT_POINTER
+criterion.protocol.statistic.str    type : FT_POINTER