-
Notifications
You must be signed in to change notification settings - Fork 47
DoubleMocks
This is a short writeup of a problem that surfaced with issue
cgreen-devs/cgreen#96
where awilke reported that mocks that
should return doubles did not work, nor did expect()
calls with double
values seem to work as expected.
(So it is not about test
doubles
but about values of C/C++ double
type, a.k.a double float.)
I'm writing this in the hope to bring some structure to this problem. Perhaps while I do that someone can give feedback so that the questions we need to answer gets clearer.
There are a number of parts to this problem
- fitting
double
s into the same value asintptr_t
, currently done withbox/unbox_double()
- returning the correct type from
mock()
, currently handled by returning whatever the user said - signaling types of parameters to
mock()
so that explicit or implicit boxing can be done if required - type checking the value to
will_return()
wrt. the return type of themock()
- ditto for parameters to
mock()
Solving 4 and 5 would give the user fair warnings and errors instead of segmentation violation
when a non-double value is unboxed (which is the case now).
The core problem is that Cgreen's internal mechanisms need to cater for
all possible (allowed) types of values. Initially only int
and
pointer
(string
is also a pointer) was supported and by using the largest integer
available, pointers and integers could live in the same variable and
casting could be done to do the right thing.
Further more, the return-value-constraint could simply take an
intptr_t
since that was always a useful return value that could be
cast into whatever the mocked function should return.
Enters double
, which does not fit into the same memory space as
intptr_t
, and it certainly cannot be cast from and to one. The
solution selected was a boxed_double
meaning a dynamically allocated
space harboring the double, and since the space could be identified by
the pointer to it, that pointer would fit into the same space as other
expected values or return values.
So a function that should return a double
could then be mocked using
expect(func, will_return(box_double(3.1415926));
and in the mocked function you should have
double func(int a) { return unbox_double(mock(a)); }
This puts the burdon of boxing/unboxing explicitly on the user.
But it works. Until either of the box
-calls are
forgotten. Then serious problems arise since as there is no foolproof way
to distinguish a pointer (to the boxed double) from an integer value.
segfault
is often the result. And there is no way to trap this and
give a useful warning/error so the user is mostly in the dark about what
caused the problem. And after debugging it is still not easy to deduce
what to do about the illegal pointer causing the problem in the midst of
Cgreen core code/macros.
In addition there is a problem with mock()
in the respect that arguments
are also assumed to fit into intptr_t
. This creates an additional problem
when passing double
as an argument. If we are following the same line of
thought, then a user should, if possible, be protected against the packing/boxing
of doubles.
However, this seems to me to be completely impossible. So, a solution should,
if possible take the same route for mock()
arguments, mock()
return values
and expected values. The current "user is responsible for boxing/unboxing"
strategy is, in a way, consistent in this respect.
Again, a primary goal should be to protect the user from serious problems when forgetting this.
Since Cgreen internally is quite "type ignorant" (everything fits into an intptr_t
,
remember) there is no way for any of _Cgreen_´s internal functions to do anything different
depending of the actual type of a value that should be compared, matched or returned.
This also means that there is no way to do "type checking" of the provided constraint match
and a parameter that has been mock()
ed.
Also, as pointed out by @awilke in his comment,
Cgreen can't copy the return value, which is required, if it is a boxed value, since it does
not know that it is. This results e.g. in problems with always_return()
of double
s.
From the user perspective it would seem natural that Cgreen would be able to return double values from mock()
and to handle any problems with storing values and verifying their types under the hood.
Unfortunately that is not possible for a number of reasons, C's type conversion system for one. One suggestion would then be that the C language user of Cgreen would need to explicitly express his/her intent for using doubles. (As, in fact, it already is with the explicit boxing/unboxing.)
With that in mind a possible introduction of
mock_double()
and
will_return_double()
would, in my mind, be a reasonable exception to the clean, single syntax that
Cgreen have. (And we already have assert_that_double()
).
An advantage this has over the explicit "user boxing/unboxing" of return values
is that it could possibly also package a "type check" with reasonable cost in
complexity. This would be valuable if we could trap the mistake discussed above,
in this case using mock_double()
with will_return()
instead of will_return_double()
and vice versa.
This would address problems 1 and 4.
If we stay with "user is responsible for boxing of parameters to mock()" there
is actually a way to handle "type checking". Since "box_double()" is explicitly
stripped from the arguments to mock()
to get the actual parameter name (that is
required in the constraints) Cgreen actually could know which are supposed to be
of type double.
This would address problems 3 and 5.
One way to address this would be to fit the values, not into an
intptr_t
, but into a CgreenValue
struct which could both harbour all
possible value types but also be tagged with the type. E.g.
typedef enum {INTEGER, STRING, DOUBLE, POINTER} CgreenValueType;
typedef struct {
CgreenValueType type;
union {
intptr_t integer_value;
double double_value;
void *pointer_value;
const char *string_value;
} value;
} CgreenValue;
(An equivalent solution would be to have this as a dynamically allocated area pointed to by e.g. constraint->expected_value
, in effect boxing, not only doubles, but all values. But since this only changes the struct
to a pointer it makes little difference, I think.)
Implementation-wise this would function by having the core
implementation of mock_()
(which is the function actually called once
the macro mock()
has done its magic with the argument list) return a
CgreenValue
instead of an intptr_t
. It could then be "unpacked" and
type checked in the mock
macro. Something along the lines of
#define mock(...) { CgreenValue c = mock_(...); <verify type and return correct value> }
The <verify type and return correct value>
should in principle be
if (c.type != <expected type>)
cgreen_error("Mismatched type in mock() return value");
return c.value.<expected type>;
Unfortunately, the only way to make the expected type known at compile
time, in C, AFAIK, is by forcing the user to use different mock()
functions/macros. In the example above:
double func(int a) { return mock_double(...); }
This would mean that we could replace mock()
with
mock_<type>()
as in
mock_double()
mock_string()
mock_integer()
mock_pointer()
This does not feel particularly good... It is complex and not in the spirit of
Cgreen's elegant "single syntax" principle. So the suggestion above, to only introduce mock_double()
seems like a reasonable trade-off.
My C++ is very rusty, and I have not done such intricate things in C++, but I believe it would be possible to use some overloading or proxy magic based on the return type. Don't know if that is feasible with the mock()
variadic parameter list, though. I suspect this is what @matthargett mentioned in his comment.
Furthermore the stripping of box_double()
from the actual argument list (in text form) that is currently done to
create a list of names would need to return a list of parameter information structs holding the name and if the
user supplied box_double()
, implying that that argument should be handled as a double, or not.
This information would then need to be inspected when matching expect
-constraints to be able to do the type checking.
The approach above points to a need to
- store various types of values in the same space
- type-tag the values
- verify type before using it
I did a refactoring of the constraints expected value from intptr_t
to CgreenValue
as suggested above on my cgreen_value
branch. It was pure joy to do it with the quick feedback of all the tests! And it seemed to work out fine. But that experiment has halted at exactly at the point where a CgreenValue
returned from mock_()
should be unpacked into a double
.
Introducing mock_double()
would require another mock_table
, or some generalization of the perl script so that it can take the name of the function as a parameter.
We would need an indirection in mock_double()
and mock()
that verifies the type of the returned boxed/packed/tagged value.
#define mock() { verify_and_return_non_double(mock_()); }
#define mock_double() { verify_and_return_double(mock_()); }
An alternative might possibly still be to continue using the "user is responsible for boxing/unboxing" strategy, if it was possible to catch user mistakes, give good error messages, and, above all, not crash.
A prerequisite to this strategy is a design that can distinguish between a pointer to BoxedDouble
and intptr_t
.