Merge pull request #1008 from jgonzalezdr/output-parameters-bug-fix

basvodde · web-flow · commit 2ed186935eee · 2016-06-23T10:39:28.000+08:00
Bug fix for wrong output parameters being copied
diff --git a/include/CppUTestExt/MockExpectedCallsList.h b/include/CppUTestExt/MockExpectedCallsList.h
@@ -43,8 +43,6 @@ class MockExpectedCallsList
     virtual int amountOfExpectationsFor(const SimpleString& name) const;
     virtual int amountOfUnfulfilledExpectations() const;
     virtual bool hasUnfulfilledExpectations() const;
-    virtual bool hasFulfilledExpectations() const;
-    virtual bool hasFulfilledExpectationsWithoutIgnoredParameters() const;
     virtual bool hasUnfulfilledExpectationsBecauseOfMissingParameters() const;
     virtual bool hasExpectationWithName(const SimpleString& name) const;
     virtual bool hasCallsOutOfOrder() const;
diff --git a/src/CppUTestExt/MockActualCall.cpp b/src/CppUTestExt/MockActualCall.cpp
@@ -105,13 +105,17 @@ void MockCheckedActualCall::finalizeOutputParameters(MockCheckedExpectedCall* ex
 
 void MockCheckedActualCall::finalizeCallWhenFulfilled()
 {
-    if (unfulfilledExpectations_.hasFulfilledExpectationsWithoutIgnoredParameters()) {
-        finalizeOutputParameters(unfulfilledExpectations_.getOneFulfilledExpectationWithIgnoredParameters());
-    }
+	// Expectations that don't ignore parameters have higher fulfillment preference than those that ignore parameters
 
-    if (unfulfilledExpectations_.hasFulfilledExpectations()) {
-        fulfilledExpectation_ = unfulfilledExpectations_.removeOneFulfilledExpectation();
+    fulfilledExpectation_ = unfulfilledExpectations_.removeOneFulfilledExpectation();
+    if (fulfilledExpectation_) {
+        finalizeOutputParameters(fulfilledExpectation_);
         callHasSucceeded();
+    } else {
+    	MockCheckedExpectedCall* fulfilledExpectationWithIgnoredParameters = unfulfilledExpectations_.getOneFulfilledExpectationWithIgnoredParameters();
+    	if (fulfilledExpectationWithIgnoredParameters) {
+    		finalizeOutputParameters(fulfilledExpectationWithIgnoredParameters);
+    	}
     }
 }
 
diff --git a/src/CppUTestExt/MockExpectedCallsList.cpp b/src/CppUTestExt/MockExpectedCallsList.cpp
@@ -81,19 +81,6 @@ int MockExpectedCallsList::amountOfUnfulfilledExpectations() const
     return count;
 }
 
-bool MockExpectedCallsList::hasFulfilledExpectations() const
-{
-    return (size() - amountOfUnfulfilledExpectations()) != 0;
-}
-
-bool MockExpectedCallsList::hasFulfilledExpectationsWithoutIgnoredParameters() const
-{
-    for (MockExpectedCallsListNode* p = head_; p; p = p->next_)
-        if (p->expectedCall_->isFulfilledWithoutIgnoredParameters())
-            return true;
-    return false;
-}
-
 bool MockExpectedCallsList::hasUnfulfilledExpectations() const
 {
     return amountOfUnfulfilledExpectations() != 0;
diff --git a/tests/CppUTestExt/ExpectedFunctionsListTest.cpp b/tests/CppUTestExt/ExpectedFunctionsListTest.cpp
@@ -63,7 +63,7 @@ TEST_GROUP(MockExpectedCallsList)
 TEST(MockExpectedCallsList, emptyList)
 {
     CHECK(! list->hasUnfulfilledExpectations());
-    CHECK(! list->hasFulfilledExpectations());
+    CHECK(list->size() == list->amountOfUnfulfilledExpectations());
     LONGS_EQUAL(0, list->size());
 }
 
diff --git a/tests/CppUTestExt/MockParameterTest.cpp b/tests/CppUTestExt/MockParameterTest.cpp
@@ -678,6 +678,29 @@ TEST(MockParameterTest, outputParameterWithIgnoredParameters)
     mock().actualCall("foo").withOutputParameter("bar", &retval).withParameter("other", 1);
 
     LONGS_EQUAL(param, retval);
+
+    mock().checkExpectations();
+}
+
+/*
+ * This test checks that the proper output parameters are copied when multiple calls to the same
+ * function are expected.
+ */
+TEST(MockParameterTest, properOutputParametersAreCopied)
+{
+    int expectedValue1 = 1;
+    int expectedValue2 = 2;
+    mock().expectOneCall("foo").withOutputParameterReturning("param", &expectedValue1, sizeof(expectedValue1)).ignoreOtherParameters();
+    mock().expectOneCall("foo").withOutputParameterReturning("param", &expectedValue2, sizeof(expectedValue2));
+
+    int returnedValue1 = 0;
+    int returnedValue2 = 0;
+    mock().actualCall("foo").withOutputParameter("param", &returnedValue1);
+    mock().actualCall("foo").withOutputParameter("param", &returnedValue2).withParameter("optional", 50);
+
+    CHECK_EQUAL_TEXT(expectedValue2, returnedValue1, "Wrong output value in 1st call");
+    CHECK_EQUAL_TEXT(expectedValue1, returnedValue2, "Wrong output value in 2nd call");
+
     mock().checkExpectations();
 }
 

Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ TEST_GROUP(MockExpectedCallsList)`
`63`	`63`	`TEST(MockExpectedCallsList, emptyList)`
`64`	`64`	`{`
`65`	`65`	`CHECK(! list->hasUnfulfilledExpectations());`
`66`		`- CHECK(! list->hasFulfilledExpectations());`
	`66`	`+ CHECK(list->size() == list->amountOfUnfulfilledExpectations());`
`67`	`67`	`LONGS_EQUAL(0, list->size());`
`68`	`68`	`}`
`69`	`69`