[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] call ordering
[Thread Prev] | [Thread Next]
[Date Prev] | [Date Next]
- Subject: Re: [PATCH] call ordering
- From: Andreas Schneider <asn@xxxxxxxxxxxxxx>
- Date: Thu, 12 Nov 2015 08:23:11 +0100
- To: cmocka@xxxxxxxxxxxxxx
- Cc: Joseph Ates <joey@xxxxxxxxxxxxxx>
On Friday 09 October 2015 15:13:00 Joseph Ates wrote: > Hello Andreas, > > I revised the original patch to reflect the new name changes and > included the missing test file. Hi Joseph, the patch looks good. Could you pleas generate the patch using git format-patch -1 --stdout > call_ordering.patch and attach it so I can get it into the tree? Please make a separate commit for the tests! Sorry for the delay. Thanks, -- andreas > > 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 -- Andreas Schneider GPG-ID: CC014E3D www.cryptomilk.org asn@xxxxxxxxxxxxxx
Re: [PATCH] call ordering | Joseph Ates <joey@xxxxxxxxxxxxxx> |