ThrowTheSwitch / CMock

CMock - Mock/stub generator for C
http://throwtheswitch.org
MIT License
671 stars 273 forks source link

ReturnThrPtr,ReturnArrayThrPtr,ReturnMemThrPtr #473

Closed joseph-eglu closed 5 months ago

joseph-eglu commented 5 months ago

I am new to cmock. When I use this (ReturnThrPtr,ReturnArrayThrPtr,ReturnMemThrPtr) and when i try to expect a function with this pointer it is only checking the value at pointer location not the the entire array or memory.This is actually wrong as the data maybe wrong.And yet the test passes. I dont know my understanding is correct but the way i understood this is happening,I will attach my test case.


    uint8_t buf[1024] = {0};
uint8_t buf1[1024] = {0, 1};

Sn_Read_File_ExpectAndReturn(myDb.pFile_name_list[0], NULL, 1024, 0, 0);
Sn_Read_File_IgnoreArg_pData();
Sn_Read_File_ReturnArrayThruPtr_pData(buf1, 1024);

memcpy((buf1 + (Fr.row_id * sizeof(DbRow_t))), &row, sizeof(DbRow_t));

Sn_Update_File_ExpectAndReturn(myDb.pFile_name_list[0], buf1, 1024, 0, 0);

TEST_ASSERT_EQUAL(0, modify_row(&myDb, Fr, (uint8_t *)&row));

this test is passing but actually if it was checking the whole buf1 in Sn_Update_File_ExpectAndReturn the the test should fail

Letme commented 5 months ago

Quick glance: if I remember correctly the IgnoreArg and ReturnThru need to be before Expect... Maybe?

mvandervoord commented 5 months ago

Hi @joseph-eglu !

Welcome to the ThrowTheSwitch.org community. Thanks for giving our tools a try!

@Letme is a great resource on these forums and I rely on him regularly to take some of the burden of answering questions. He's rarely wrong, so I almost hate to point out that, in this case, he is. ;) The order of your statements is correct in this case.

I have a suspicion I know what is happening, but let's walk through what your test is doing:

// You set up two buffers for this test. I notice that buf never gets used. I suspect that might be unintentional
uint8_t buf[1024] = {0};
uint8_t buf1[1024] = {0, 1};

// This tells CMock to Expect a call to `Sn_Read_File`. 
// When it happens, it should return a 0. It'll also check all these arguments... until...
Sn_Read_File_ExpectAndReturn(myDb.pFile_name_list[0], NULL, 1024, 0, 0);

// This line tells CMock that you want to ignore the pData argument and NOT check that the data is correct
Sn_Read_File_IgnoreArg_pData();

// Finally, this tells CMock that you want to copy 1K of data through pData 
Sn_Read_File_ReturnArrayThruPtr_pData(buf1, 1024);

// Here we manipulate buf1 in some weird way and using values that aren't in this test. 
// I suspect this is supposed to mimic what happens to the buffer inside modify_row?
// POTENTIAL PROBLEM: you're manipulating buf1 here, but the Sn_Read_File call above queues a POINTER, 
// not the data, therefore THIS is the value you're giving to the function now. 
// PERHAPS: maybe you intended to use buf instead of buf1 for this line and the line below?
// SIDENOTE: tests are cleaner when they don't directly mimic the operations in the file being tested... it might 
// be cleaner to hardcode the compare string, if that's not too hard?
memcpy((buf1 + (Fr.row_id * sizeof(DbRow_t))), &row, sizeof(DbRow_t));

// Here you're expecting that when Sn_Update_File gets called, it gets called with the contents of buf1.
// PROBLEM: If you intend this to check 1024 bytes, you want _ExpectWithArrayAndReturn instead.
// POTENTIAL PROBLEM: as mentioned above, you're using the same buffer here (buf1) which 
// means that you've told CMock to use the same CONTENTS. 
Sn_Update_File_ExpectAndReturn(myDb.pFile_name_list[0], buf1, 1024, 0, 0);

TEST_ASSERT_EQUAL(0, modify_row(&myDb, Fr, (uint8_t *)&row));

I hope this helps! If not, feel free to ask more questions and give us a bit more detail (The specific failure you're getting and the function prototypes that are getting mocked are both helpful).

Mark

Letme commented 5 months ago

@mvandervoord I checked my tests and I was indeed wrong. You expect and then ignoreargs/returnthruptr. I guess I got used to this oddity, as otherwise in my mind you always prepare arguments first, then call the Expect (function). But I long understood where it came from and today my thinking is aligned enough to think my previous thinking was wrong.

Another side note which helped me a lot. Keep in mind Expect's do not run the function, do not store pointed stuff anywhere, so at the time of function modify_row call it takes the buffers as they are then - not how you manipulated them before. This is helpful when you call single function multiple times, but expect different values returned through pointer of returned array (communication protocol). For that case, you cannot overwrite the existing buffer, but like Mark said, you need to prepare messages (strings, arrays, whatever) upfront and just pass pointers to correct messages.

In most cases do not modify existing buffers, if for nothing else, for test readability and maintainability. Takes out the guessing work which "variation" of buffer was meant to pass the test.

mvandervoord commented 5 months ago

Well said, @Letme :)

joseph-eglu commented 5 months ago

Thank you for the reply @mvandervoord . It fixed my issue.