[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] call ordering


Hello Andreas,

I revised the original patch to reflect the new name changes and
included the missing test file.

Thanks for the feedback,
Joseph

Signed-off-by: Joseph Ates <joseph.ates@xxxxxxxxxxxxx>
---
 include/cmocka.h      | 142 +++++++++++++++++++++++++++++++++++++++
 src/cmocka.c          | 160 ++++++++++++++++++++++++++++++++++++++++----
 src/cmocka.def        |   2 +
 tests/CMakeLists.txt  |  11 +++-
 tests/test_ordering.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 481 insertions(+), 13 deletions(-)
 create mode 100644 tests/test_ordering.c

diff --git a/include/cmocka.h b/include/cmocka.h
index 72e4e5e..45df169 100644
--- a/include/cmocka.h
+++ b/include/cmocka.h
@@ -149,6 +149,7 @@
cast_to_largest_integral_type(cast_to_pointer_integral_type(value))
 #define CMOCKA_DEPRECATED
 #endif

+
 /**
  * @defgroup cmocka_mock Mock Objects
  * @ingroup cmocka
@@ -1336,6 +1337,138 @@ void assert_not_in_set(LargestIntegralType
value, LargestIntegralType values[],
 /** @} */

 /**
+ * @defgroup cmocka_call_order Call Ordering
+ * @ingroup cmocka
+ *
+ * It is often beneficial to  make sure that functions are called in an
+ * order. This is independent of mock returns and parameter checking as both
+ * of the aforementioned do not check the order in which they are called from
+ * different functions.
+ *
+ * <ul>
+ * <li><strong>expect_function_call(function)</strong> - The
+ * expect_function_call() macro pushes an expectation onto the stack of
+ * expected calls.</li>
+ *
+ * <li><strong>function_called()</strong> - pops a value from the stack of
+ * expected calls. function_called() is invoked within the mock object
+ * that uses it.
+ * </ul>
+ *
+ * expect_function_call() and function_called() are intended to be used in
+ * pairs. Cmocka will fail a test if there are more or less expected calls
+ * created (e.g. expect_function_call()) than consumed with function_called().
+ * There are provisions such as ignore_function_calls() which allow this
+ * restriction to be circumvented in tests where mock calls for the code under
+ * test are not the focus of the test.
+ *
+ * The following example illustrates how a unit test instructs cmocka
+ * to expect a function_called() from a particular mock,
+ * <strong>chef_sing()</strong>:
+ *
+ * @code
+ * void chef_sing(void);
+ *
+ * void code_under_test()
+ * {
+ *   chef_sing();
+ * }
+ *
+ * void some_test(void **state)
+ * {
+ *     expect_function_call(chef_sing);
+ *     code_under_test();
+ * }
+ * @endcode
+ *
+ * The implementation of the mock then must check whether it was meant to
+ * be called by invoking <strong>function_called()</strong>:
+ *
+ * @code
+ * void chef_sing()
+ * {
+ *     function_called();
+ * }
+ * @endcode
+ *
+ * @{
+ */
+
+#ifdef DOXYGEN
+/**
+ * @brief Check that current mocked function is being called in the expected
+ *        order
+ *
+ * @see expect_function_call()
+ */
+void function_called(void);
+#else
+#define function_called() _function_called(__func__, __FILE__, __LINE__)
+#endif
+
+#ifdef DOXYGEN
+/**
+ * @brief Store expected call(s) to a mock to be checked by function_called()
+ *        later.
+ *
+ * @param[in]  #function  The function which should should be called
+ *
+ * @param[in]  times number of times this mock must be called
+ *
+ * @see function_called()
+ */
+void expect_function_calls(#function, const int times);
+#else
+#define expect_function_calls(function, times) \
+    _expect_function_call(#function, __FILE__, __LINE__, times)
+#endif
+
+#ifdef DOXYGEN
+/**
+ * @brief Store expected single call to a mock to be checked by
+ *        function_called() later.
+ *
+ * @param[in]  #function  The function which should should be called
+ *
+ * @see function_called()
+ */
+void expect_function_call(#function);
+#else
+#define expect_function_call(function) \
+    _expect_function_call(#function, __FILE__, __LINE__, 1)
+#endif
+
+#ifdef DOXYGEN
+/**
+ * @brief Expects function_called() from given mock at least once
+ *
+ * @param[in]  #function  The function which should should be called
+ *
+ * @see function_called()
+ */
+void expect_function_call_any(#function);
+#else
+#define expect_function_call_any(function) \
+    _expect_function_call(#function, __FILE__, __LINE__, -1)
+#endif
+
+#ifdef DOXYGEN
+/**
+ * @brief Ignores function_called() invocations from given mock function.
+ *
+ * @param[in]  #function  The function which should should be called
+ *
+ * @see function_called()
+ */
+void ignore_function_calls(#function);
+#else
+#define ignore_function_calls(function) \
+    _expect_function_call(#function, __FILE__, __LINE__, -2)
+#endif
+
+/** @} */
+
+/**
  * @defgroup cmocka_exec Running Tests
  * @ingroup cmocka
  *
@@ -1938,6 +2071,15 @@ extern const char * global_last_failed_assert;
 LargestIntegralType _mock(const char * const function, const char* const file,
                           const int line);

+void _expect_function_call(
+    const char * const function_name,
+    const char * const file,
+    const int line,
+    const int count);
+
+void _function_called(const char * const function, const char* const file,
+                          const int line);
+
 void _expect_check(
     const char* const function, const char* const parameter,
     const char* const file, const int line,
diff --git a/src/cmocka.c b/src/cmocka.c
index a4dbae8..3e09194 100644
--- a/src/cmocka.c
+++ b/src/cmocka.c
@@ -174,6 +174,12 @@ typedef struct SymbolMapValue {
     ListNode symbol_values_list_head;
 } SymbolMapValue;

+/* Where a particular ordering was located and its symbol name */
+typedef struct OrderingValue {
+    SourceLocation location;
+    const char * function;
+} OrderingValue;
+
 /* Used by list_free() to deallocate values referenced by list nodes. */
 typedef void (*CleanupListValue)(const void *value, void *cleanup_value_data);

@@ -229,9 +235,16 @@ static void free_symbol_map_value(
     const void *value, void *cleanup_value_data);
 static void remove_always_return_values(ListNode * const map_head,
                                         const size_t number_of_symbol_names);
+
+static int check_for_leftover_values_list(const ListNode * head,
+    const char * const error_message);
+
 static int check_for_leftover_values(
     const ListNode * const map_head, const char * const error_message,
     const size_t number_of_symbol_names);
+
+static void remove_always_return_values_from_list(ListNode * const map_head);
+
 /*
  * This must be called at the beginning of a test to initialize some data
  * structures.
@@ -274,6 +287,11 @@ static CMOCKA_THREAD ListNode
global_function_parameter_map_head;
 /* Location of last parameter value checked was declared. */
 static CMOCKA_THREAD SourceLocation global_last_parameter_location;

+/* List (acting as FIFO) of call ordering. */
+static CMOCKA_THREAD ListNode global_call_ordering_head;
+/* Location of last call ordering that was declared. */
+static CMOCKA_THREAD SourceLocation global_last_call_ordering_location;
+
 /* List of all currently allocated blocks. */
 static CMOCKA_THREAD ListNode global_allocated_blocks;

@@ -401,17 +419,19 @@ static void set_source_location(

 /* Create function results and expected parameter lists. */
 void initialize_testing(const char *test_name) {
-    (void)test_name;
+    (void)test_name;
     list_initialize(&global_function_result_map_head);
     initialize_source_location(&global_last_mock_value_location);
     list_initialize(&global_function_parameter_map_head);
     initialize_source_location(&global_last_parameter_location);
+    list_initialize(&global_call_ordering_head);
+    initialize_source_location(&global_last_parameter_location);
 }


 static void fail_if_leftover_values(const char *test_name) {
     int error_occurred = 0;
-    (void)test_name;
+    (void)test_name;
     remove_always_return_values(&global_function_result_map_head, 1);
     if (check_for_leftover_values(
             &global_function_result_map_head,
@@ -425,6 +445,12 @@ static void fail_if_leftover_values(const char
*test_name) {
             "%s parameter still has values that haven't been checked.\n", 2)) {
         error_occurred = 1;
     }
+
+    remove_always_return_values_from_list(&global_call_ordering_head);
+    if (check_for_leftover_values_list(&global_call_ordering_head,
+        "%s function was expected to be called but was not not.\n")) {
+        error_occurred = 1;
+    }
     if (error_occurred) {
         exit_test(1);
     }
@@ -432,13 +458,16 @@ static void fail_if_leftover_values(const char
*test_name) {


 static void teardown_testing(const char *test_name) {
-    (void)test_name;
+    (void)test_name;
     list_free(&global_function_result_map_head, free_symbol_map_value,
               (void*)0);
     initialize_source_location(&global_last_mock_value_location);
     list_free(&global_function_parameter_map_head, free_symbol_map_value,
               (void*)1);
     initialize_source_location(&global_last_parameter_location);
+    list_free(&global_call_ordering_head, free_value,
+              (void*)0);
+    initialize_source_location(&global_last_call_ordering_location);
 }

 /* Initialize a list node. */
@@ -557,7 +586,7 @@ static int list_first(ListNode * const head,
ListNode **output) {

 /* Deallocate a value referenced by a list. */
 static void free_value(const void *value, void *cleanup_value_data) {
-    (void)cleanup_value_data;
+    (void)cleanup_value_data;
     assert_non_null(value);
     free((void*)value);
 }
@@ -585,7 +614,6 @@ static int symbol_names_match(const void
*map_value, const void *symbol) {
                    (const char*)symbol);
 }

-
 /*
  * Adds a value to the queue of values associated with the given hierarchy of
  * symbols.  It's assumed value is allocated from the heap.
@@ -675,6 +703,26 @@ static int get_symbol_value(
     return 0;
 }

+/**
+ * Taverse a list of nodes and remove first symbol value in list that has a
+ * refcount < -1 (i.e. should always be returned and has been returned at
+ * least once).
+ */
+
+static void remove_always_return_values_from_list(ListNode * const map_head)
+{
+    ListNode * current = NULL;
+    ListNode * next = NULL;
+    assert_non_null(map_head);
+
+    for (current = map_head->next, next = current->next;
+            current != map_head;
+            current = next, next = current->next) {
+        if (current->refcount < -1) {
+            list_remove_free(current, free_value, NULL);
+        }
+    }
+}

 /*
  * Traverse down a tree of symbol values and remove the first symbol value
@@ -714,6 +762,26 @@ static void remove_always_return_values(ListNode
* const map_head,
     }
 }

+static int check_for_leftover_values_list(const ListNode * head,
+                                          const char * const error_message)
+{
+    ListNode *child_node;
+    int leftover_count = 0;
+    if (!list_empty(head))
+    {
+        for (child_node = head->next; child_node != head;
+                 child_node = child_node->next, ++leftover_count) {
+            const OrderingValue *const o =
+                    (const OrderingValue*) child_node->value;
+            cm_print_error(error_message, o->function);
+            cm_print_error(SOURCE_LOCATION_FORMAT
+                    ": note: remaining item was declared here\n",
+                    o->location.file, o->location.line);
+        }
+    }
+    return leftover_count;
+}
+
 /*
  * Checks if there are any leftover values set up by the test that were never
  * retrieved through execution, and fail the test if that is the case.
@@ -790,13 +858,60 @@ LargestIntegralType _mock(const char * const
function, const char* const file,
     return 0;
 }

+/* Ensure that function is being called in proper order */
+void _function_called(const char * const function, const char* const file,
+    const int line)
+{
+    ListNode *first_value_node = NULL;
+    ListNode *value_node = NULL;
+    OrderingValue * expected_call;
+    int rc = list_first(&global_call_ordering_head, &value_node);
+    first_value_node = value_node;
+    if (rc) {
+        expected_call = (OrderingValue *)value_node->value;
+        if (value_node->refcount < -1) {
+            /* search through value nodes until either function is found or
+               encounter a non-zero refcount greater than -2 */
+            if (strcmp(expected_call->function, function)) {
+                value_node = value_node->next;
+                expected_call = (OrderingValue *)value_node->value;
+                while (value_node->refcount < -1 &&
+                        strcmp(expected_call->function, function) &&
+                        value_node != first_value_node->prev) {
+                    value_node = value_node->next;
+                    expected_call = (OrderingValue *)value_node->value;
+                }
+                if (value_node == first_value_node->prev) {
+                    cm_print_error(SOURCE_LOCATION_FORMAT ": error:
No expected mock calls matching called() "
+                        "invocation in %s", file, line, function);
+                    exit_test(1);
+                }
+            }
+        }
+        if (!strcmp(expected_call->function, function)) {
+            if (value_node->refcount > -2 && --value_node->refcount == 0) {
+                list_remove_free(value_node, NULL, NULL);
+            }
+        }
+        else {
+            cm_print_error(SOURCE_LOCATION_FORMAT ": error: Expected
call to %s but "
+                "received called() in %s\n", file, line,
expected_call->function, function);
+            exit_test(1);
+        }
+    }
+    else {
+        cm_print_error(SOURCE_LOCATION_FORMAT ": error: No mock calls
expected but "
+            "called() was invoked in %s\n", file, line, function);
+        exit_test(1);
+    }
+}

 /* Add a return value for the specified mock function name. */
 void _will_return(const char * const function_name, const char * const file,
                   const int line, const LargestIntegralType value,
                   const int count) {
     SymbolValue * const return_value =
-        (SymbolValue*)malloc(sizeof(*return_value));
+        (SymbolValue*)malloc(sizeof(*return_value));
     assert_true(count > 0 || count == -1);
     return_value->value = value;
     set_source_location(&return_value->location, file, line);
@@ -828,6 +943,27 @@ void _expect_check(
                      count);
 }

+/*
+ * Add an call expectations that a particular function is called correctly.
+ * This is used for code under test that makes calls to several functions
+ * in depended upon components (mocks).
+ */
+
+void _expect_function_call(
+    const char * const function_name,
+    const char * const file,
+    const int line,
+    const int count)
+{
+    OrderingValue * const ordering = (OrderingValue*)malloc(sizeof(*ordering));
+    assert_non_null(function_name);
+    assert_non_null(file);
+    assert_true(count != 0);
+    set_source_location(&ordering->location, file, line);
+    ordering->function = function_name;
+
+    list_add_value(&global_call_ordering_head, ordering, count);
+}

 /* Returns 1 if the specified values are equal.  If the values are not equal
  * an error is displayed and 0 is returned. */
@@ -1234,7 +1370,7 @@ static void expect_memory_setup(
         const void * const memory, const size_t size,
         const CheckParameterValue check_function, const int count) {
     CheckMemoryData * const check_data =
-        (CheckMemoryData*)malloc(sizeof(*check_data) + size);
+    (CheckMemoryData*)malloc(sizeof(*check_data) + size);
     void * const mem = (void*)(check_data + 1);
     declare_initialize_value_pointer_pointer(check_data_pointer, check_data);
     assert_non_null(memory);
@@ -1266,7 +1402,7 @@ static int check_not_memory(const
LargestIntegralType value,
     assert_non_null(check);
     return memory_not_equal_display_error(
         cast_largest_integral_type_to_pointer(const char*, value),
-    (const char*)check->memory,
+        (const char*)check->memory,
         check->size);
 }

@@ -1284,8 +1420,8 @@ void _expect_not_memory(
 /* CheckParameterValue callback that always returns 1. */
 static int check_any(const LargestIntegralType value,
                      const LargestIntegralType check_value_data) {
-    (void)value;
-    (void)check_value_data;
+    (void)value;
+    (void)check_value_data;
     return 1;
 }

@@ -1734,7 +1870,7 @@ static int display_allocated_blocks(const
ListNode * const check_point) {

     for (node = check_point->next; node != head; node = node->next) {
         const MallocBlockInfo * const block_info =
-        (const MallocBlockInfo*)node->value;
+        (const MallocBlockInfo*)node->value;
         assert_non_null(block_info);

         if (!allocated_blocks) {
@@ -2762,7 +2898,7 @@ int _run_tests(const UnitTest * const tests,
const size_t number_of_tests) {
      * when a test setup occurs and popped on tear down.
      */
     TestState* test_states =
-        (TestState*)malloc(number_of_tests * sizeof(*test_states));
+    (TestState*)malloc(number_of_tests * sizeof(*test_states));
     /* The number of test states which should be 0 at the end */
     long number_of_test_states = 0;
     /* Names of the tests that failed. */
diff --git a/src/cmocka.def b/src/cmocka.def
index 607ad75..7c740a6 100644
--- a/src/cmocka.def
+++ b/src/cmocka.def
@@ -12,9 +12,11 @@ EXPORTS
     _assert_string_equal
     _assert_string_not_equal
     _assert_true
+    _called
     _check_expected
     _cmocka_run_group_tests
     _expect_any
+    _expect_call
     _expect_check
     _expect_in_range
     _expect_in_set
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 2f985a5..90e92c5 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -16,7 +16,8 @@ set(CMOCKA_TESTS
     test_exception_handler
     test_basics
     test_skip
-    test_setup_fail)
+    test_setup_fail
+    test_ordering)

 foreach(_CMOCKA_TEST ${CMOCKA_TESTS})
     add_cmocka_test(${_CMOCKA_TEST} ${_CMOCKA_TEST}.c ${CMOCKA_STATIC_LIBRARY})
@@ -46,6 +47,14 @@ set_tests_properties(
         "\\[  FAILED  \\] 1 test"
 )

+# test_ordering ensure proper failures
+set_tests_properties(
+    test_ordering
+        PROPERTIES
+        PASS_REGULAR_EXPRESSION
+        "\\[  FAILED  \\] 7 test"
+)
+
 # test_exception_handler
 if (WIN32)
     set_tests_properties(
diff --git a/tests/test_ordering.c b/tests/test_ordering.c
new file mode 100644
index 0000000..172bed9
--- /dev/null
+++ b/tests/test_ordering.c
@@ -0,0 +1,179 @@
+#include "config.h"
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include <cmocka_private.h>
+
+static void mock_test_a_called(void)
+{
+    function_called();
+}
+
+static void mock_test_b_called(void)
+{
+    function_called();
+}
+
+static void mock_test_c_called(void)
+{
+    function_called();
+}
+
+static void test_does_fail_for_unexpected_call(void **state)
+{
+    (void)state;
+    expect_function_call(mock_test_a_called);
+    expect_function_call(mock_test_a_called);
+
+    mock_test_a_called();
+    mock_test_a_called();
+    mock_test_a_called();
+}
+
+
+static void test_does_fail_for_unmade_expected_call(void **state)
+{
+    (void)state;
+    expect_function_call(mock_test_a_called);
+    expect_function_call(mock_test_a_called);
+
+    mock_test_a_called();
+}
+
+
+static void test_does_succeed_for_expected(void **state)
+{
+    (void)state;
+    expect_function_call(mock_test_a_called);
+    expect_function_call(mock_test_a_called);
+
+    mock_test_a_called();
+    mock_test_a_called();
+}
+
+static void test_does_succeed_for_multiple_calls(void **state)
+{
+    (void)state;
+    expect_function_call(mock_test_a_called);
+    expect_function_calls(mock_test_a_called, 2);
+    expect_function_call(mock_test_a_called);
+
+    mock_test_a_called();
+    mock_test_a_called();
+    mock_test_a_called();
+    mock_test_a_called();
+}
+
+static void test_ordering_does_ignore_calls(void **state)
+{
+    (void)state;
+
+    ignore_function_calls(mock_test_a_called);
+
+    mock_test_a_called();
+    mock_test_a_called();
+    mock_test_a_called();
+}
+
+static void test_ordering_does_ignore_no_calls(void **state)
+{
+    (void)state;
+    ignore_function_calls(mock_test_a_called);
+}
+
+static void test_ordering_does_expect_at_least_one_call(void **state)
+{
+    (void)state;
+    expect_function_call_any(mock_test_a_called);
+
+    mock_test_a_called();
+    mock_test_a_called();
+    mock_test_a_called();
+}
+
+static void test_ordering_does_work_across_different_functions(void **state)
+{
+    (void)state;
+    expect_function_call(mock_test_a_called);
+    expect_function_call(mock_test_b_called);
+    expect_function_call(mock_test_a_called);
+
+    mock_test_a_called();
+    mock_test_b_called();
+    mock_test_a_called();
+}
+
+static void test_ordering_fails_out_of_order(void **state)
+{
+    (void)state;
+    expect_function_call(mock_test_a_called);
+    expect_function_call(mock_test_b_called);
+    expect_function_call(mock_test_a_called);
+
+    mock_test_b_called();
+}
+
+static void test_ordering_ignores_out_of_order_properly(void **state)
+{
+    (void)state;
+    ignore_function_calls(mock_test_a_called);
+    ignore_function_calls(mock_test_b_called);
+    expect_function_calls(mock_test_c_called, 2);
+
+
+    mock_test_c_called();
+    mock_test_b_called();
+    mock_test_c_called();
+}
+
+static void test_ordering_fails_out_of_order_for_at_least_once_calls(void
**state)
+{
+    (void)state;
+    expect_function_call_any(mock_test_a_called);
+    ignore_function_calls(mock_test_b_called);
+
+    mock_test_b_called();
+    mock_test_c_called();
+}
+
+/* Primarily used to test error message */
+static void test_fails_out_of_order_if_no_calls_found_on_any(void **state)
+{
+    (void)state;
+    expect_function_call_any(mock_test_a_called);
+    ignore_function_calls(mock_test_b_called);
+
+    mock_test_a_called();
+    mock_test_c_called();
+}
+
+static void test_fails_if_zero_count_used(void **state)
+{
+    (void)state;
+    expect_function_calls(mock_test_a_called, 0);
+
+    mock_test_a_called();
+}
+
+int main(void) {
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_does_fail_for_unexpected_call)
+        ,cmocka_unit_test(test_does_fail_for_unmade_expected_call)
+        ,cmocka_unit_test(test_does_fail_for_unmade_expected_call)
+        ,cmocka_unit_test(test_does_succeed_for_expected)
+        ,cmocka_unit_test(test_does_succeed_for_multiple_calls)
+        ,cmocka_unit_test(test_ordering_does_ignore_no_calls)
+        ,cmocka_unit_test(test_ordering_does_ignore_calls)
+        ,cmocka_unit_test(test_ordering_does_expect_at_least_one_call)
+        ,cmocka_unit_test(test_ordering_does_work_across_different_functions)
+        ,cmocka_unit_test(test_ordering_fails_out_of_order)
+        ,cmocka_unit_test(test_ordering_ignores_out_of_order_properly)
+        ,cmocka_unit_test(test_ordering_fails_out_of_order_for_at_least_once_calls)
+        ,cmocka_unit_test(test_fails_out_of_order_if_no_calls_found_on_any)
+        ,cmocka_unit_test(test_fails_if_zero_count_used)
+    };
+
+    return cmocka_run_group_tests(tests, NULL, NULL);
+}
--
1.9.1


References:
[PATCH] call orderingJoseph Ates <joey@xxxxxxxxxxxxxx>
Re: [PATCH] call orderingAndreas Schneider <asn@xxxxxxxxxxxxxx>