-
Notifications
You must be signed in to change notification settings - Fork 514
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding call-checking when tracing is on. #453
base: master
Are you sure you want to change the base?
Conversation
Added an alternative tracing class, MockCheckedActualCallTrace, derived from MockCheckedActualCall. (The class, MockActualCallTrace, is still present, but its use with the tracing feature is disabled.) Access to a few attributes of MockCheckedActualCall were given to the new class through protected methods, to support the feature. The MockSupport class was modified to use the new trace class in place of the former one. Tests in MockSupportTest.cpp were enhanced to cover the tracing feature more completely. A small correction to MockNamedValue (completing initialization) was done; afaik this is not related to the tracing feature, but was found during code analysis and appeared to be generally beneficial.
Wow @sjstraw, thanks! We'll need to work a bit on the code and tests though. As is, I won't be merging, but we can work with it. Will that be good for you? The first big question I have is:
Then, some other suggestions for improvement:
To move forward. You mentioned a bug in MockNamedValue. Could we do that one first? Then we'll move from there with the discussion and with making this smaller. Will that work for you? |
Sure. Tracing gives a chronological look at the actual calls. The error reports from the expected calls describes conditions in terms of, well, expectations. It is often helpful to have a clear view of what-happened-in-what-order to understand where expectations and reality parted ways. As you mentioned to me in the past, tracing is useful to understand a lesser-understood piece of code, in a black-box sort of way. If I give it stimulus A, what actual calls does it produce? How about stimulus ... B? Checking becomes useful (possibly - at least in my eyes) when you want to take what you observed in tracing and make sure that the CUT continues to act according to your observations. Though one should avoid writing brittle code, once the tracing is done, there are a set of assumptions that will be used to write software that utilizes that CUT, and you don't want those assumptions to be broken. So you trace and observe what was called. Once you know what is called, with your growing understanding, you expect certain behavior, which you add to the test. With call checking enabled with tracing, you have the opportunity to build up these expected calls while tracing. That is the basis of my reason to attempt combining the two. |
I will reduce the degree of comments. If code is unclear, I will refactor to make it so. Yes, the extended test is large, and covers many use cases. I will break it up. TDD was used extensively during the development of the code. I currently don't have a piece of legacy code to test-drive the changes on, other than the tests in MockSupportTest.cpp. Re: "Doesn't this class now duplicate a lot with the other tracing functionality? Can we do it so that it doesn't do that?" The only way I found to combine tracing and call checking was to derive the tracing class off of MockCheckedActualCall. The intention was that the new class would not duplicate, but would replace MockActualCallTrace. Your question reveals that I didn't yet come up with a way to still allow tracing without checking; that will have to be thought through. |
Scott, Could you implement tracing by 'wrapping' an existing actual call Ryan On Tue, Dec 23, 2014, 3:54 PM Scott Straw notifications@github.com wrote:
|
Hmm, but @sjstraw... the problem is that when you use tracing and checking, when a test fails then the test won't continue anymore and neither will the tracing. So that is why tracing and checking is rarely useful as the first unexpected call would end the test and there is no trace :) |
Exactly. So with tracing as it is now you can either trace, or you can add expectations. You cannot do both. By deriving from MockCheckedActualCall, I gained the ability to disable the failing of the test. The "failures" still are sent to the reporter, but the test continues. I believe it was done by overriding failtest(). What you have with tracing as it is now may be sufficient for everyone's needs. There are certainly ways around the issues I encountered. I ran into a desire to have both tracing and checking together when I was part way through adding expectations for a test, and wanted to determine in what order the next few actual calls were being executed. I don't remember all the specifics now, but it seemed that I ran into difficulties when turning on tracing in my test that already had expectations. My intention with this pull request was to allow the opportunity to turn tracing on and off at will while analyzing code behavior and adding expectations. If there is enough added value for the users of CppUMock in this, then let us continue. Otherwise, our time is precious and we should use it on other things with greater gain.
|
Ah ok, so it doesn't actually do checking but it does the failure reporting ? Will it still mark the test as failed? But it will continue to run? Thanks for your patience, I think I'm beginning to understand it :) |
Yes, you have the right idea. It goes through the mechanics of checking calls and parameters, but if the expectations are not met, they are reported as a "MOCK TRACE" instead of a "MOCK FAILURE", and the call that would cause the test to fail is not done. I believe I ended up grabbing some code from the function that is usually called to exit the test, so I could report the failure without exiting the test. Will it still mark the test as failed? Thanks for the question! I believe that is the case, but it should be tested. I will add one. Thanks again for your interest in this. BTW, I will move the small change to MockNamedValue off on a separate pull request, since it is unrelated.
|
Ok, so then the big question is whether the old tracing is still useful, right? Your thought is not... and I guess so too. Can we think of any reason to also keep the old tracing? Another design comment, already made by @ryanplusplus, it might be better to do a decorator than to subclass from MockCheckedActualCall. Do you think that could also work? |
Added an alternative tracing class, MockCheckedActualCallTrace, derived
from MockCheckedActualCall. (The class, MockActualCallTrace, is still
present, but its use with the tracing feature is disabled.) Access to a
few attributes of MockCheckedActualCall were given to the new class
through protected methods, to support the feature.
The MockSupport class was modified to use the new trace class in place
of the former one.
Tests in MockSupportTest.cpp were enhanced to cover the tracing feature
more completely.
A small correction to MockNamedValue (completing initialization) was
done; as far as I know this is not related to the tracing feature, but was found
during code analysis and appeared to be generally beneficial.