On Fri, Mar 01, 2013 at 02:38:08PM +0100, Pavel Březina wrote:
On 03/01/2013 02:01 PM, Jakub Hrozek wrote:
>On Fri, Mar 01, 2013 at 01:39:22PM +0100, Pavel Březina wrote:
>>>[1]
http://cmocka.cryptomilk.org/
>>
>>Very nice.
>>
>>I would like to increase granularity though. First of all we should
>>separate cmocka test files from check files on directory/filename
>>level. I'd say that all cmocka stuff should go into tests/cmocka. For
>>example, there are files common_check.c, common_dom.c, common.c, but
>>only common_dom.c is suitable for cmocka. When more common files and
>>tests comes, it will be hard to distinguish them.
>>
>>Testing with cmocka is mainly about mocking structures and creating
>>wrappers around functions. We should try to create reusable mocks and
>>wrappers right from the beginning, making them independent on tests.
>>For example: mock_rctx(), mock_cctx(), sss_dp_get_account_send(),
>>sss_dp_get_account_recv(), ... I believe these and more functions can
>>be reused when testing other responders so they should be placed in
>>e.g. common_responder.c
>>
>
>Definitely. As I said in the original mail, I didn't want to spend too
>much time without getting some kind of feedback.
>
>I will move the definitions to some common file and resubmit.
>
>>Why did you include setjmp.h?
>
>I will add a comment to clarify this.
OK, I went through the test code and here are few ideas to improve
readability.
A wrapper has the ability to execute the original function. And you use
will_return(__wrap_fn, true/false) to choose the code flow. A macro
would be better:
will_return(__wrap_sss_packet_get_body, CALL_REAL);
will_return(__wrap_sss_packet_get_body, CALL_WRAPPER);
or even better:
call_real(fn)
call_wrapper(fn)
and we should standardize wrappers to always have this ability and that
it should be set before any other will_return() is called.
I was experimenting with a wrapper like this but it looked cumbersome to
me. Mostly because you also need to declare the __real function and if
the wrapper was in a separate module, both wrapper and real function
would have to be in the header file. This looked quite ugly to me. I
would rather keep this suggestion in mind and see in time when other
tests land if they would also need this feature.
Also will_return() macro is used to set both parameters and return
value. It would be nice to distinguish between those too on semantic
level. Something like:
mock_param(fn, value) /* we are setting parameter */
will_return(fn, value) /* we are setting return value */
It would be good if we can provider an order parameter to mock_param()
and mock() so we don't have to rely on the calling order. But this
probably needs changes in cmocka.
Yes, this sounds like a good idea if it can be done from cmocka's point
of view.
Create a macro for:
*body = (uint8_t *) mock() => mock_as(uint8_t*)
OK, done.
New tests attached.