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 — ignoreAdditionalCalls (ignoreMultipleCalls?) by maxilai · Pull Request #973 · cpputest/cpputest

Conversation

@maxilai

@maxilai maxilai commented May 19, 2016

Copy link
Copy Markdown
Contributor

Hi, basvodde and arstrube
My colleagues raised an requirements for "ignore additional (multiple, extra) calls".
Here, we have ignoreOtherCalls which does not ignore the expected one.
Then, we add a feature ignoreAdditionalCalls which behaviors like below. What I concern is the name "ignoreAdditionalCalls". Is this name a good one?
Thanks!

TEST(MockCallTest, ignoreAdditionalCallsExceptForTheExpectedOne)
{
mock().expectOneCall("foo");
mock().ignoreAdditionalCalls();
mock().actualCall("foo");

/* The following calls will not fail, because the additional calls for expected is ignored. */
mock().actualCall("foo");    
mock().actualCall("foo").withParameter("foo", 1);

mock().clear();

}

Sorry, something went wrong.

Uh oh!

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

All reactions
@maxilai
ignoreAdditionalCalls (ignoreMultipleCalls?)
7231844
@coveralls

coveralls commented May 20, 2016

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.0005%) to 99.874% when pulling 7231844 on maxilai:issue_ignoreAdditionalCalls into 7ea6f14 on cpputest:master.

All reactions

Sorry, something went wrong.

Uh oh!

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

@basvodde

basvodde commented May 20, 2016

Copy link
Copy Markdown
Member

I'm somewhat uncomfortable with this feature as I don't know in what case you would want to use it. Could you explain that to me? It feels it opens the door to making a lot weaker tests.

All reactions

Sorry, something went wrong.

Uh oh!

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

@maxilai

maxilai commented May 20, 2016 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Thanks for your feedback !

The thing is. In they test cases, they call a mocked function many times.
They don't care how many times the calling happens. They just care
the behaviors of production code as expected based the returned value from
this mocked function.
In the production code, they will call this mocked function many times.
like below:
while (condition)
{
...
importantValue = p->foo(parameters); // Here, the mocked funciton will
be called.
...
otherImportantValue = whatEverFunctions();
condition = caculatingCondition(importantValue, otherImportantValue);
}
Here, they just want to call the mocked function in order to get a
important value. But they don't care how many times this mock function are
called.
When they make some change from whatEverFunction() or any other related
functions. The times of happening of the mocked function will change. They
have to change the test code to revise the parameter amount from
expectNCalls method for lots of test cases. So , they raise this
requirement.

With ignoreOtherCalls and ignoreAdditionCalls, programmers will be able to
have cpputest to only check what they specified by expectedOne(N/No)Calls.
They want 5 calls, the cpputest will only check the first 5 calls, ignore
the 6th and 7th etc.

Or, do you guys have any good suggestion?

Thanks!

2016-05-20 9:44 GMT+08:00 Bas Vodde [email protected]:

I'm somewhat uncomfortable with this feature as I don't know in what case
you would want to use it. Could you explain that to me? It feels it opens
the door to making a lot weaker tests.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cpputest_cpputest_pull_973-23issuecomment-2D220497329&d=CwMCaQ&c=q3cDpHe1hF8lXU5EFjNM_A&r=FVxCMeE3ZYygueFGtwMh7hYD5ccLcrx20IyPZhBafNI&m=YJ4dpoZRX5r5cNcAkDupWrs4HMClsbMfhSfR20jo7rI&s=lem2AUJc5oIeVMzIltvAyGattrKuqPLGWQTxClV2A3Y&e=

All reactions

Sorry, something went wrong.

Uh oh!

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

@arstrube

arstrube commented May 20, 2016

Copy link
Copy Markdown
Contributor

As a matter of interest, how do they determine pass / fail? I assume they have some way to check the value of condition in the test?

One thing I don't really understand is, if the first N calls influence the value of condition, how come the rest of the calls don't? This seems kind of strange to me.

Would it solve their problem if they set default return values for these mocks and then used disable() - enable()` to switch of mocking altogether, while the function under test is executed?

All reactions

Sorry, something went wrong.

Uh oh!

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

@basvodde

basvodde commented May 20, 2016

Copy link
Copy Markdown
Member

@maxilai It feels a bit like a hack to patch poorly written unit tests, so I'm very hesitant to add it. Even if we would add it, the way it is suggested now is generic so that it is for all expectations. That would make even less sense as you mentioned the ignoreAdditional calls ought to be per function.

I'd love to hear others opinion, but for me so far, it feels a bit wrong. Perhaps you could come up with an actual concrete test, so we could look at that?

All reactions

Sorry, something went wrong.

Uh oh!

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

@maxilai

maxilai commented May 20, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for the advice from all of you.
I will contact with my colleagues to get more information about their test cases. Thanks again.

All reactions

Sorry, something went wrong.

Uh oh!

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

@jgonzalezdr

jgonzalezdr commented Jul 10, 2016

Copy link
Copy Markdown

As a matter of fact, this behavior can be achieved with the functionality added in PR #1018, but in a more controlled way expectation by expectation instead of allowing all the expectations to get additional calls (which might be inadequate for some tests).

All reactions

Sorry, something went wrong.

Uh oh!

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

@cuitoldfish cuitoldfish mentioned this pull request Jul 15, 2016
Expected calls matching several actual calls #1018
Open

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.

5 participants

@maxilai @coveralls @basvodde @arstrube @jgonzalezdr
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.