Skip to content
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

ignoreAdditionalCalls (ignoreMultipleCalls?) #973

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxilai
Copy link
Contributor

@maxilai maxilai commented May 19, 2016

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();

}

@coveralls
Copy link

Coverage Status

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

@basvodde
Copy link
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.

@maxilai
Copy link
Contributor Author

maxilai commented May 20, 2016

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 notifications@github.com:

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=

@arstrube
Copy link
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?

@basvodde
Copy link
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?

@maxilai
Copy link
Contributor Author

maxilai commented May 20, 2016

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

@jgonzalezdr
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants