Issue 12 in mockitopp: information on missing implementation is not actionable for developers without serious debugger time

[mock...@googlecode.com]

Status: Accepted
Owner: plaztiks...@gmail.com
Labels: Type-Enhancement Priority-Medium

New issue 12 by plaztiks...@gmail.com: information on missing
implementation is not actionable for developers without serious debugger
time
http://code.google.com/p/mockitopp/issues/detail?id=12

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.


--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[mock...@googlecode.com]

Comment #1 on issue 12 by plaztiks...@gmail.com: information on missing

implementation is not actionable for developers without serious debugger
time
http://code.google.com/p/mockitopp/issues/detail?id=12

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.

Attachments:
mockitopp.better_exception_info.patch 37.4 KB

[mock...@googlecode.com]

Comment #2 on issue 12 by plaztiks...@gmail.com: information on missing

implementation is not actionable for developers without serious debugger
time
http://code.google.com/p/mockitopp/issues/detail?id=12

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! :)

Attachments:
mockitopp.better_exception_info.patch 235 KB

[mock...@googlecode.com]

Comment #3 on issue 12 by trevor.p...@gmail.com: information on missing

implementation is not actionable for developers without serious debugger
time

https://code.google.com/p/mockitopp/issues/detail?id=12

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.