tpounds / mockitopp

Simple mocking for C++
MIT License
77 stars 6 forks source link

information on missing implementation is not actionable for developers without serious debugger time #12

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. call a mocked function one too many times on a line with multiple mocked 
calls

mock_object<interface> mock;
mock_object<other> other_mock;
if (mock.getInstance().f() && other_mock.getInstance().g()) {}

2. a missing_implementation_exception() is thrown with no details about which 
class/member is involved

this is presenting a major usability problem, requiring disassembly-level 
debugging on some platforms.

What is the expected output? What do you see instead?
the exception ideally would have the member name in the what() of the 
exception. while there's no platform-agnostic way to do this, glibc offers 
dladdr() that returns a struct containing the symbol name in the dli_sname 
field.

non-ideally, the typeinfo() or method signature ((bool)interface::*) being in 
the what() would provide enough of a clue in most cases.

I have some ideas for ways to modify dynamic_object to do some of this, but 
want to get other ideas as well.

Original issue reported on code.google.com by plaztiks...@gmail.com on 7 Mar 2013 at 9:55

GoogleCodeExporter commented 9 years ago
Here is a sketch for one possible approach: getting the typeinfo at runtime.

It initializes an array of typeinfo (that correlates to the vtable offsets) 
with the typeinfo for the mocked class by default. As the mock has expectations 
set on specific members, the array is filled in with the more specific 
typeinfo. (It "learns" as you program the mock.) I don't know of a way to fill 
in the (R)C::*() typeinfo ahead of time, even minimally -- that would be 
awesome to figure out.

This simple approach is already a huge leap forward. Now when a mock is 
misused, I see this in my test runner output insteead of just 
"[std::exception]":

protocol/http/appblade/http_cfe_blade_test.cc:66: Failure: http_cfe_blade_tests 
-> does_not_create_appblade_when_request_parsing_not_done 
    an exception was thrown during test: [partial_implementation_exception: bool (Message::*)()]

protocol/http/appblade/http_cfe_blade_test.cc:85: Failure: http_cfe_blade_tests 
-> gives_parsed_request_to_appblade_returned_by_manager 
    an exception was thrown during test: [missing_implementation_exception: Request]

The top failure is an example of when an expectation is set and the member 
information is therefore more detailed. The latter failure is an example of 
when an expectation for the mock wasn't set, and is therefore less detailed. 
Even with the less detail, I can look at the test and see which mock supplied 
to my class's ctor needs additional expectations without a debugger.

An alternate approach would be to use a macro (like cgreen), which would allow 
us just to capture the text for a given member:
#define expect(mock, member) mock.expect(member, #member)
// ...
void test(void) { mock_object<foo> mock_foo; expect(mock_foo, 
foo::method).when().thenReturn(); }

macros violate namespaces, but it may be worth it if we can't fill in more 
typeinfo ahead of time.

The next step is to have the partial_implementation_exception provide more 
information about which argument/constraint failed comparison.

Original comment by plaztiks...@gmail.com on 13 Mar 2013 at 1:53

Attachments:

GoogleCodeExporter commented 9 years ago
an updated patch:
1) print out the (streamable) arguments supplied to the mock that failed
2) identify the specific argument that didn't match

protocol/http/appblade/http_cfe_blade_test.cc:66: Failure: http_cfe_blade_tests 
-> does_not_create_appblade_when_request_parsing_not_done 
    an exception was thrown during test: [partial_implementation_exception: bool (Message::*)(DataBuf const*, unsigned int) {  [???],  1!!! }]

in the output above, the first argument isn't printable. the second argument is 
"1", which doesn't match the expectation -- hence the "!!!". it took me days of 
messing with m4 to get something working, and I'm not happy with the structure. 
obviously some cleanups needs to happen since the dynamic_object and 
dynamic_vfunction files are now cluttered a bit.

it would be nice to also include what the when() expectations were -- 
optimizing away the need to look in the test sources, but I don't know how to 
get the Matcher subclass information. it comes out of the tuple as the base 
class only.

I'll test in VC++2012 in a day or two. I'd like to test in clang, but mockitopp 
in svn doesn't compile with latest clang (a separate bug I filed).

comments very welcome! :)

Original comment by plaztiks...@gmail.com on 19 Mar 2013 at 1:39

Attachments:

GoogleCodeExporter commented 9 years ago
I think this patch really makes a lot of sense to improve overall usability. My 
biggest concern is the use of typeid which requires RTTI be enabled. The patch 
as is will not compile if -fno-rtti is specified on the command line.

Original comment by trevor.p...@gmail.com on 7 Sep 2014 at 6:03

matthargett commented 9 years ago

Is it acceptable if these features are disabled when RTTI isn't present? Is there a way to reliably test that across compilers at compile-time (preferred) or run-time (ugh)?