PengZheng commented on a change in pull request #392:
URL: https://github.com/apache/celix/pull/392#discussion_r791780927
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -76,7 +76,7 @@ celix_status_t bundleContext_create(framework_pt framework,
celix_framework_logg
}
}
-
+ //FIXME: context == NULL?
Review comment:
I guess you are talking about injecting errors into libc or any 3rd
party library function call when doing unit test, right?
The standard mocking method in C++ is to provide mock implementation of an
abstract interface. The C equivalent is using function pointers:
So instead of
```C
int FormatOutput(const char *, ...);
```
we put the following in the header file:
```C
extern int (*FormatOutput)(const char *, ...);
```
Though both of them support runtime-bound test doubles very well, careless
use can lead to lots of unnecessary abstract classes/function pointers, which
could harm both performance and code readability considerably. In this
particular case (libc/3rd party C libraries), these runtime test doubles are
pretty awkward to use, i.e. we have to re-exposing functions as function
pointers instead of using header file directly.
Instead, I use link-time substitution. It does not require any wierd
wrappers in production codes. We still need wrappers, but they only appear in
testing code. It permits testing corner cases which is almost impossible to
cover in system test like the following:
```C
TEST(AlarmUploader, HeartbeatTimeoutWhenSending)
{
NETRET_HEADER header;
expectRecv = true;
//enable link-time test doubles for clock_gettime and gettimeofday
//if disabled, they will call the libc's implementation
EnableClockMock(true);
LONGS_EQUAL(EDA_OK, AlarmUploaderInit(conn, module, &attr));
LONGS_EQUAL(EDA_OK, AlarmUploaderConnect(conn, NULL, NULL));
//enable IO mock to substitute write and writev, which libuv uses for
socket IO
EnableIoMock(true);
//limit each write/writev invocation can send out at most 1 byte
//so that the transportation of tiny heartbeat packet (< 100 bytes)
cannot be completed in one uv_run call
SetIoMockMaxBytesPerWrite(1);
// let 5.001 seconds pass in our fake clock to trigger 5 seconds
timeout
ClockAdvance(5001);
// actually trigger libuv timer with heartbeat pending
uv_run(loop, UV_RUN_NOWAIT);
// disable IO mock, so that libuv send out packet at normal speed
EnableIoMock(false);
ClockAdvance(5000);
ExpectNetRetHeader(&header, NETRET_QUALIFIED);
mock().expectOneCall("message_received").withUnsignedIntParameter("status",
NETRET_EXCHANGE);
mock().expectOneCall("message_received").withUnsignedIntParameter("status",
NETRET_EXCHANGE);
uv_run(loop, UV_RUN_NOWAIT);
ObjectPolyFinish((Object)conn);
fd = -1;
EnableIoMock(false);
}
```
Triggering timeout when sending a tiny packet is very difficult (if not
possible) in reality, even with the help of expensive network emulation
hardware. But with link-time test double, it's easy to inject various errors
into any C library call. Also the test case is extremely fast to execute, since
we use fake clock to avoid waiting 10 seconds. High line/branch coverage (like
sqlite) is not difficult to achieve.
However, such powerful testing technique is not without its price:
* 1000 LOC `AlarmUploader` takes 3300 LOC to test.
* Such tests are not written against API, they are completely whitebox.
* Bad readability as a consequence. API test case can be used as demo while
the above test case cannot.
We have to take extra care of the interplay between malloc/calloc/pthread
test double and Google Address Sanitizer so that they can work together. I'll
prepare a gist for malloc/calloc if Celix wants to take this whitebox way of
testing. @pnoltes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
