GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Adding call-checking when tracing is on. by sjstraw · Pull Request #453 · cpputest/cpputest

Conversation

@sjstraw

@sjstraw sjstraw commented Dec 22, 2014

Copy link
Copy Markdown

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sjstraw
Adding call-checking when tracing is on.
fb126ec
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.
@basvodde

basvodde commented Dec 23, 2014

Copy link
Copy Markdown
Member

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:

  • Why would you want call checking when tracing? What is the use case this came up?

Then, some other suggestions for improvement:

  • The code has an awful lot of comments. In general, we prefer not to comment things that are obvious. If it is not obvious, we prefer to make it obvious. If that isn't possible, then add a comment.
  • The test you extended is now very big? It seems you added many tests in one test?
  • Doesn't this class now duplicate a lot with the other tracing functionality? Can we do it so that it doesn't do that?
  • Did you test-drive the code? Might be good to practice TDD on it?

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sjstraw

sjstraw commented Dec 23, 2014

Copy link
Copy Markdown
Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sjstraw

sjstraw commented Dec 23, 2014

Copy link
Copy Markdown
Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ryanplusplus

ryanplusplus commented Dec 23, 2014

Copy link
Copy Markdown
Contributor

Scott,

Could you implement tracing by 'wrapping' an existing actual call
implementation? To do this you'd intercept and then forward all calls to
the 'wrapped' object. Upon intercepting the calls, you would trace the
calls. To enable checking, you'd wrap the normal implementation and to
disable checking you'd pass in an implementation that does nothing.

Ryan

On Tue, Dec 23, 2014, 3:54 PM Scott Straw [email protected] wrote:

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.

—
Reply to this email directly or view it on GitHub
#453 (comment).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@basvodde

basvodde commented Dec 24, 2014

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sjstraw

sjstraw commented Dec 24, 2014

Copy link
Copy Markdown
Author

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.

On Dec 24, 2014, at 2:42 AM, Bas Vodde [email protected] 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 :)

—
Reply to this email directly or view it on GitHub.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@basvodde

basvodde commented Dec 24, 2014

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sjstraw

sjstraw commented Dec 24, 2014

Copy link
Copy Markdown
Author

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.

On Dec 24, 2014, at 10:12 AM, Bas Vodde [email protected] wrote:

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

—
Reply to this email directly or view it on GitHub.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@basvodde

basvodde commented Dec 25, 2014

Copy link
Copy Markdown
Member

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sjstraw @basvodde @ryanplusplus
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.