Make Wire tests more strict w.r.t. expectations

This changes Wire tests to produces errors on "uninteresting calls" and
flushes mock expectations on client/server flushes so that we control
100% of the order of calls.

BUG=941543

Change-Id: I6eabc79dde2abb564e54df90c5c0e615fd4496c6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6040
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

generator/templates/mock_api.cpp

@@ -14,6 +14,8 @@
 
 #include "mock_dawn.h"
 
+using namespace testing;
+
 namespace {
     {% for type in by_category["object"] %}
         {% for method in native_methods(type) if len(method.arguments) < 10 %}
@@ -128,3 +130,9 @@ void ProcTableAsClass::CallFenceOnCompletionCallback(DawnFence fence,
 
 MockProcTable::MockProcTable() {
 }
+
+void MockProcTable::IgnoreAllReleaseCalls() {
+    {% for type in by_category["object"] %}
+        EXPECT_CALL(*this, {{as_MethodSuffix(type.name, Name("release"))}}(_)).Times(AnyNumber());
+    {% endfor %}
+}

generator/templates/mock_api.h

@@ -101,6 +101,8 @@ class MockProcTable : public ProcTableAsClass {
     public:
         MockProcTable();
 
+        void IgnoreAllReleaseCalls();
+
         {% for type in by_category["object"] %}
             {% for method in type.methods if len(method.arguments) < 10 %}
                 MOCK_METHOD{{len(method.arguments) + 1}}(

src/tests/unittests/wire/WireArgumentTests.cpp

@@ -40,8 +40,6 @@ TEST_F(WireArgumentTests, ValueArgument) {
 
     EXPECT_CALL(api, ComputePassEncoderDispatch(apiPass, 1, 2, 3)).Times(1);
 
-    EXPECT_CALL(api, CommandEncoderRelease(apiEncoder));
-    EXPECT_CALL(api, ComputePassEncoderRelease(apiPass));
     FlushClient();
 }
 
@@ -72,8 +70,6 @@ TEST_F(WireArgumentTests, ValueArrayArgument) {
     EXPECT_CALL(api,
                 ComputePassEncoderSetPushConstants(apiPass, DAWN_SHADER_STAGE_BIT_VERTEX, 0, 4,
                                                    ResultOf(CheckPushConstantValues, Eq(true))));
-    EXPECT_CALL(api, CommandEncoderRelease(apiEncoder));
-    EXPECT_CALL(api, ComputePassEncoderRelease(apiPass));
 
     FlushClient();
 }
@@ -168,8 +164,6 @@ TEST_F(WireArgumentTests, CStringArgument) {
                         return desc->vertexStage->entryPoint == std::string("main");
                     })))
         .WillOnce(Return(nullptr));
-    EXPECT_CALL(api, ShaderModuleRelease(apiVsModule));
-    EXPECT_CALL(api, PipelineLayoutRelease(apiLayout));
 
     FlushClient();
 }
@@ -196,9 +190,6 @@ TEST_F(WireArgumentTests, ObjectAsValueArgument) {
     dawnCommandEncoderCopyBufferToBuffer(cmdBufEncoder, buffer, 0, buffer, 4, 4);
     EXPECT_CALL(api, CommandEncoderCopyBufferToBuffer(apiEncoder, apiBuffer, 0, apiBuffer, 4, 4));
 
-    EXPECT_CALL(api, CommandEncoderRelease(apiEncoder));
-    EXPECT_CALL(api, BufferRelease(apiBuffer));
-
     FlushClient();
 }
 
@@ -222,8 +213,6 @@ TEST_F(WireArgumentTests, ObjectsAsPointerArgument) {
         apiCmdBufs[i] = api.GetNewCommandBuffer();
         EXPECT_CALL(api, CommandEncoderFinish(apiCmdBufEncoder))
             .WillOnce(Return(apiCmdBufs[i]));
-        EXPECT_CALL(api, CommandEncoderRelease(apiCmdBufEncoder));
-        EXPECT_CALL(api, CommandBufferRelease(apiCmdBufs[i]));
     }
 
     // Create queue
@@ -239,7 +228,6 @@ TEST_F(WireArgumentTests, ObjectsAsPointerArgument) {
                              return cmdBufs[0] == apiCmdBufs[0] && cmdBufs[1] == apiCmdBufs[1];
                          })));
 
-    EXPECT_CALL(api, QueueRelease(apiQueue));
     FlushClient();
 }
 
@@ -302,7 +290,6 @@ TEST_F(WireArgumentTests, StructureOfObjectArrayArgument) {
                          })))
         .WillOnce(Return(nullptr));
 
-    EXPECT_CALL(api, BindGroupLayoutRelease(apiBgl));
     FlushClient();
 }
 
@@ -339,7 +326,6 @@ TEST_F(WireArgumentTests, StructureOfStructureArrayArgument) {
             })))
         .WillOnce(Return(apiBgl));
 
-    EXPECT_CALL(api, BindGroupLayoutRelease(apiBgl));
     FlushClient();
 }
 

src/tests/unittests/wire/WireBasicTests.cpp

@@ -32,7 +32,6 @@ TEST_F(WireBasicTests, CallForwarded) {
     EXPECT_CALL(api, DeviceCreateCommandEncoder(apiDevice))
         .WillOnce(Return(apiCmdBufEncoder));
 
-    EXPECT_CALL(api, CommandEncoderRelease(apiCmdBufEncoder));
     FlushClient();
 }
 
@@ -48,8 +47,6 @@ TEST_F(WireBasicTests, CreateThenCall) {
     DawnCommandBuffer apiCmdBuf = api.GetNewCommandBuffer();
     EXPECT_CALL(api, CommandEncoderFinish(apiCmdBufEncoder)).WillOnce(Return(apiCmdBuf));
 
-    EXPECT_CALL(api, CommandEncoderRelease(apiCmdBufEncoder));
-    EXPECT_CALL(api, CommandBufferRelease(apiCmdBuf));
     FlushClient();
 }
 
@@ -63,7 +60,6 @@ TEST_F(WireBasicTests, RefCountKeptInClient) {
     DawnCommandEncoder apiCmdBufEncoder = api.GetNewCommandEncoder();
     EXPECT_CALL(api, DeviceCreateCommandEncoder(apiDevice))
         .WillOnce(Return(apiCmdBufEncoder));
-    EXPECT_CALL(api, CommandEncoderRelease(apiCmdBufEncoder));
 
     FlushClient();
 }

src/tests/unittests/wire/WireBufferMappingTests.cpp

@@ -29,7 +29,7 @@ namespace {
                           DawnCallbackUserdata userdata));
     };
 
-    std::unique_ptr<MockBufferMapReadCallback> mockBufferMapReadCallback;
+    std::unique_ptr<StrictMock<MockBufferMapReadCallback>> mockBufferMapReadCallback;
     void ToMockBufferMapReadCallback(DawnBufferMapAsyncStatus status,
                                      const void* ptr,
                                      uint32_t dataLength,
@@ -48,7 +48,7 @@ namespace {
                           DawnCallbackUserdata userdata));
     };
 
-    std::unique_ptr<MockBufferMapWriteCallback> mockBufferMapWriteCallback;
+    std::unique_ptr<StrictMock<MockBufferMapWriteCallback>> mockBufferMapWriteCallback;
     uint32_t* lastMapWritePointer = nullptr;
     void ToMockBufferMapWriteCallback(DawnBufferMapAsyncStatus status,
                                       void* ptr,
@@ -70,8 +70,8 @@ class WireBufferMappingTests : public WireTest {
     void SetUp() override {
         WireTest::SetUp();
 
-        mockBufferMapReadCallback = std::make_unique<MockBufferMapReadCallback>();
+        mockBufferMapReadCallback = std::make_unique<StrictMock<MockBufferMapReadCallback>>();
-        mockBufferMapWriteCallback = std::make_unique<MockBufferMapWriteCallback>();
+        mockBufferMapWriteCallback = std::make_unique<StrictMock<MockBufferMapWriteCallback>>();
 
         {
             DawnBufferDescriptor descriptor;
@@ -83,7 +83,6 @@ class WireBufferMappingTests : public WireTest {
             EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _))
                 .WillOnce(Return(apiBuffer))
                 .RetiresOnSaturation();
-            EXPECT_CALL(api, BufferRelease(apiBuffer));
             FlushClient();
         }
         {
@@ -107,6 +106,13 @@ class WireBufferMappingTests : public WireTest {
         mockBufferMapWriteCallback = nullptr;
     }
 
+    void FlushServer() {
+        WireTest::FlushServer();
+
+        Mock::VerifyAndClearExpectations(&mockBufferMapReadCallback);
+        Mock::VerifyAndClearExpectations(&mockBufferMapWriteCallback);
+    }
+
   protected:
     // A successfully created buffer
     DawnBuffer buffer;
@@ -194,6 +200,9 @@ TEST_F(WireBufferMappingTests, DestroyBeforeReadRequestEnd) {
         .Times(1);
 
     dawnBufferRelease(errorBuffer);
+
+    FlushClient();
+    FlushServer();
 }
 
 // Check the map read callback is called with UNKNOWN when the map request would have worked, but
@@ -309,6 +318,8 @@ TEST_F(WireBufferMappingTests, DestroyInsideMapReadCallback) {
 
     FlushServer();
 
+    EXPECT_CALL(api, BufferRelease(apiBuffer));
+
     FlushClient();
 }
 
@@ -514,5 +525,7 @@ TEST_F(WireBufferMappingTests, DestroyInsideMapWriteCallback) {
 
     FlushServer();
 
+    EXPECT_CALL(api, BufferRelease(apiBuffer));
+
     FlushClient();
 }

src/tests/unittests/wire/WireCallbackTests.cpp

@@ -25,7 +25,7 @@ namespace {
         MOCK_METHOD2(Call, void(const char* message, DawnCallbackUserdata userdata));
     };
 
-    std::unique_ptr<MockDeviceErrorCallback> mockDeviceErrorCallback;
+    std::unique_ptr<StrictMock<MockDeviceErrorCallback>> mockDeviceErrorCallback;
     void ToMockDeviceErrorCallback(const char* message, DawnCallbackUserdata userdata) {
         mockDeviceErrorCallback->Call(message, userdata);
     }
@@ -39,7 +39,7 @@ namespace {
                           DawnCallbackUserdata userdata2));
     };
 
-    std::unique_ptr<MockBuilderErrorCallback> mockBuilderErrorCallback;
+    std::unique_ptr<StrictMock<MockBuilderErrorCallback>> mockBuilderErrorCallback;
     void ToMockBuilderErrorCallback(DawnBuilderErrorStatus status,
                                     const char* message,
                                     DawnCallbackUserdata userdata1,
@@ -58,17 +58,23 @@ class WireCallbackTests : public WireTest {
     void SetUp() override {
         WireTest::SetUp();
 
-        mockDeviceErrorCallback = std::make_unique<MockDeviceErrorCallback>();
+        mockDeviceErrorCallback = std::make_unique<StrictMock<MockDeviceErrorCallback>>();
-        mockBuilderErrorCallback = std::make_unique<MockBuilderErrorCallback>();
+        mockBuilderErrorCallback = std::make_unique<StrictMock<MockBuilderErrorCallback>>();
     }
 
     void TearDown() override {
         WireTest::TearDown();
 
-        // Delete mocks so that expectations are checked
         mockDeviceErrorCallback = nullptr;
         mockBuilderErrorCallback = nullptr;
     }
+
+    void FlushServer() {
+        WireTest::FlushServer();
+
+        Mock::VerifyAndClearExpectations(&mockDeviceErrorCallback);
+        Mock::VerifyAndClearExpectations(&mockBuilderErrorCallback);
+    }
 };
 
 // Test that we get a success builder error status when no error happens
@@ -89,8 +95,6 @@ TEST_F(WireCallbackTests, SuccessCallbackOnBuilderSuccess) {
             return apiBuffer;
         }));
 
-    EXPECT_CALL(api, BufferBuilderRelease(apiBufferBuilder));
-    EXPECT_CALL(api, BufferRelease(apiBuffer));
     FlushClient();
 
     EXPECT_CALL(*mockBuilderErrorCallback, Call(DAWN_BUILDER_ERROR_STATUS_SUCCESS, _, 1, 2));
@@ -157,8 +161,6 @@ TEST_F(WireCallbackTests, SuccessCallbackNotForwardedToDevice) {
             return apiBuffer;
         }));
 
-    EXPECT_CALL(api, BufferBuilderRelease(apiBufferBuilder));
-    EXPECT_CALL(api, BufferRelease(apiBuffer));
     FlushClient();
     FlushServer();
 }
@@ -182,7 +184,6 @@ TEST_F(WireCallbackTests, ErrorCallbackForwardedToDevice) {
             return nullptr;
         }));
 
-    EXPECT_CALL(api, BufferBuilderRelease(apiBufferBuilder));
     FlushClient();
 
     EXPECT_CALL(*mockDeviceErrorCallback, Call(_, userdata)).Times(1);
@@ -239,8 +240,6 @@ TEST_F(WireCallbackTests, BuilderErrorCallback) {
             return apiBuffer;
         }));
 
-    EXPECT_CALL(api, BufferBuilderRelease(apiBufferBuilder));
-    EXPECT_CALL(api, BufferRelease(apiBuffer));
     FlushClient();
 
     // The error callback gets called on the client side

src/tests/unittests/wire/WireFenceTests.cpp

@@ -25,7 +25,7 @@ namespace {
         MOCK_METHOD2(Call, void(const char* message, DawnCallbackUserdata userdata));
     };
 
-    std::unique_ptr<MockDeviceErrorCallback> mockDeviceErrorCallback;
+    std::unique_ptr<StrictMock<MockDeviceErrorCallback>> mockDeviceErrorCallback;
     void ToMockDeviceErrorCallback(const char* message, DawnCallbackUserdata userdata) {
         mockDeviceErrorCallback->Call(message, userdata);
     }
@@ -35,7 +35,7 @@ namespace {
         MOCK_METHOD2(Call, void(DawnFenceCompletionStatus status, DawnCallbackUserdata userdata));
     };
 
-    std::unique_ptr<MockFenceOnCompletionCallback> mockFenceOnCompletionCallback;
+    std::unique_ptr<StrictMock<MockFenceOnCompletionCallback>> mockFenceOnCompletionCallback;
     void ToMockFenceOnCompletionCallback(DawnFenceCompletionStatus status,
                                          DawnCallbackUserdata userdata) {
         mockFenceOnCompletionCallback->Call(status, userdata);
@@ -52,14 +52,14 @@ class WireFenceTests : public WireTest {
     void SetUp() override {
         WireTest::SetUp();
 
-        mockDeviceErrorCallback = std::make_unique<MockDeviceErrorCallback>();
+        mockDeviceErrorCallback = std::make_unique<StrictMock<MockDeviceErrorCallback>>();
-        mockFenceOnCompletionCallback = std::make_unique<MockFenceOnCompletionCallback>();
+        mockFenceOnCompletionCallback =
+            std::make_unique<StrictMock<MockFenceOnCompletionCallback>>();
 
         {
             queue = dawnDeviceCreateQueue(device);
             apiQueue = api.GetNewQueue();
             EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue));
-            EXPECT_CALL(api, QueueRelease(apiQueue));
             FlushClient();
         }
         {
@@ -71,7 +71,6 @@ class WireFenceTests : public WireTest {
             fence = dawnQueueCreateFence(queue, &descriptor);
 
             EXPECT_CALL(api, QueueCreateFence(apiQueue, _)).WillOnce(Return(apiFence));
-            EXPECT_CALL(api, FenceRelease(apiFence));
             FlushClient();
         }
     }
@@ -79,11 +78,17 @@ class WireFenceTests : public WireTest {
     void TearDown() override {
         WireTest::TearDown();
 
-        // Delete mocks so that expectations are checked
         mockDeviceErrorCallback = nullptr;
         mockFenceOnCompletionCallback = nullptr;
     }
 
+    void FlushServer() {
+        WireTest::FlushServer();
+
+        Mock::VerifyAndClearExpectations(&mockDeviceErrorCallback);
+        Mock::VerifyAndClearExpectations(&mockFenceOnCompletionCallback);
+    }
+
   protected:
     void DoQueueSignal(uint64_t signalValue) {
         dawnQueueSignal(queue, fence, signalValue);
@@ -216,6 +221,9 @@ TEST_F(WireFenceTests, OnCompletionSynchronousValidationSuccess) {
     dawnFenceOnCompletion(fence, 2u, ToMockFenceOnCompletionCallback, 0);
     dawnFenceOnCompletion(fence, 3u, ToMockFenceOnCompletionCallback, 0);
     dawnFenceOnCompletion(fence, 4u, ToMockFenceOnCompletionCallback, 0);
+
+    EXPECT_CALL(*mockFenceOnCompletionCallback, Call(DAWN_FENCE_COMPLETION_STATUS_UNKNOWN, _))
+        .Times(3);
 }
 
 // Without any flushes, errors should be generated when waiting on a value greater
@@ -261,8 +269,6 @@ TEST_F(WireFenceTests, DestroyBeforeOnCompletionEnd) {
     EXPECT_CALL(*mockFenceOnCompletionCallback,
                 Call(DAWN_FENCE_COMPLETION_STATUS_UNKNOWN, userdata))
         .Times(1);
-
-    dawnFenceRelease(fence);
 }
 
 // Test that signaling a fence on a wrong queue is invalid
@@ -270,7 +276,6 @@ TEST_F(WireFenceTests, SignalWrongQueue) {
     DawnQueue queue2 = dawnDeviceCreateQueue(device);
     DawnQueue apiQueue2 = api.GetNewQueue();
     EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue2));
-    EXPECT_CALL(api, QueueRelease(apiQueue2));
     FlushClient();
 
     DawnCallbackUserdata userdata = 1520;
@@ -285,7 +290,6 @@ TEST_F(WireFenceTests, SignalWrongQueueDoesNotUpdateValue) {
     DawnQueue queue2 = dawnDeviceCreateQueue(device);
     DawnQueue apiQueue2 = api.GetNewQueue();
     EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue2));
-    EXPECT_CALL(api, QueueRelease(apiQueue2));
     FlushClient();
 
     DawnCallbackUserdata userdata = 1024;

src/tests/unittests/wire/WireOptionalTests.cpp

@@ -59,7 +59,6 @@ TEST_F(WireOptionalTests, OptionalObjectValue) {
                          })))
         .WillOnce(Return(nullptr));
 
-    EXPECT_CALL(api, BindGroupLayoutRelease(apiBindGroupLayout));
     FlushClient();
 }
 
@@ -187,8 +186,5 @@ TEST_F(WireOptionalTests, OptionalStructPointer) {
                     })))
         .WillOnce(Return(nullptr));
 
-    EXPECT_CALL(api, ShaderModuleRelease(apiVsModule));
-    EXPECT_CALL(api, PipelineLayoutRelease(apiLayout));
-
     FlushClient();
 }

src/tests/unittests/wire/WireTest.cpp

@@ -34,10 +34,7 @@ void WireTest::SetUp() {
 
     // This SetCallback call cannot be ignored because it is done as soon as we start the server
     EXPECT_CALL(api, OnDeviceSetErrorCallback(_, _, _)).Times(Exactly(1));
-    if (mIgnoreSetCallbackCalls) {
+    SetupIgnoredCallExpectations();
-        EXPECT_CALL(api, OnBuilderSetErrorCallback(_, _, _, _)).Times(AnyNumber());
-    }
-    EXPECT_CALL(api, DeviceTick(_)).Times(AnyNumber());
 
     mS2cBuf = std::make_unique<utils::TerribleCommandBuffer>();
     mC2sBuf = std::make_unique<utils::TerribleCommandBuffer>(mWireServer.get());
@@ -62,13 +59,24 @@ void WireTest::TearDown() {
     // be reset before any mocks are deleted.
     // Incomplete client callbacks will be called on deletion, so the mocks
     // cannot be null.
+    api.IgnoreAllReleaseCalls();
     mWireClient = nullptr;
 }
 
 void WireTest::FlushClient() {
     ASSERT_TRUE(mC2sBuf->Flush());
+
+    Mock::VerifyAndClearExpectations(&api);
+    SetupIgnoredCallExpectations();
 }
 
 void WireTest::FlushServer() {
     ASSERT_TRUE(mS2cBuf->Flush());
 }
+
+void WireTest::SetupIgnoredCallExpectations() {
+    if (mIgnoreSetCallbackCalls) {
+        EXPECT_CALL(api, OnBuilderSetErrorCallback(_, _, _, _)).Times(AnyNumber());
+    }
+    EXPECT_CALL(api, DeviceTick(_)).Times(AnyNumber());
+}

src/tests/unittests/wire/WireTest.h

@@ -85,11 +85,12 @@ class WireTest : public testing::Test {
     void FlushClient();
     void FlushServer();
 
-    MockProcTable api;
+    testing::StrictMock<MockProcTable> api;
     DawnDevice apiDevice;
     DawnDevice device;
 
   private:
+    void SetupIgnoredCallExpectations();
     bool mIgnoreSetCallbackCalls = false;
 
     std::unique_ptr<dawn_wire::WireServer> mWireServer;