Mir

Merge lp:~kdub/mir/nicemock-improvements into lp:~mir-team/mir/trunk

  1. nicemock-improvements
  2. Merge into trunk
Proposed by Kevin DuBois
Status: Merged
Approved by: Kevin DuBois
Approved revision: no longer in the source branch.
Merged at revision: 611
Proposed branch: lp:~kdub/mir/nicemock-improvements
Merge into: lp:~mir-team/mir/trunk
Diff against target: 257 lines (+31/-31)
10 files modified
tests/unit-tests/client/android/test_client_surface_interpreter.cpp (+11/-11)
tests/unit-tests/client/test_client_mir_surface.cpp (+1/-1)
tests/unit-tests/graphics/android/test_android_display_factory.cpp (+4/-4)
tests/unit-tests/graphics/android/test_framebuffer_factory.cpp (+1/-1)
tests/unit-tests/graphics/android/test_hwc_device.cpp (+2/-2)
tests/unit-tests/graphics/android/test_server_interpreter.cpp (+2/-2)
tests/unit-tests/graphics/egl_mock/hw_mock.cpp (+2/-2)
tests/unit-tests/graphics/test_graphics_platform.cpp (+1/-1)
tests/unit-tests/shell/test_application_session.cpp (+5/-5)
tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp (+2/-2)
To merge this branch: bzr merge lp:~kdub/mir/nicemock-improvements
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Robert Ancell Approve
Review via email: mp+159465@code.launchpad.net

Commit message

there were some chatty unit tests that needed silencing via NiceMock.
NiceMock's destructors also did not provide noexcept guarantee (needed for some android interfaces). Improve gmock's NiceMock so the Nicemock/StrictMock destructors are noexcept(true), since they don't throw anyways

Description of the change

there were some chatty unit tests that needed silencing via NiceMock.
NiceMock's destructors also did not provide noexcept guarantee (needed for some android interfaces). Improve gmock's NiceMock so the Nicemock/StrictMock destructors are noexcept(true), since they don't throw anyways. sent the gmock patch upstream as well, we'll see if they accept it!

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:670
http://jenkins.qa.ubuntu.com/job/mir-ci/417/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/424
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/307
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/422
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/422/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/200

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/417/rebuild

review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) :
review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Is it correct to be changing a "generated" file?

Also, I suspect it would be safer to use the more verbose:

    noexcept(noexcept(((MockClass*)0)->~MockClass()))

on the destructor definitions. (Isn't there an easier way to say that?)

review: Needs Information
Revision history for this message
Kevin DuBois (kdub) wrote :

> Is it correct to be changing a "generated" file?
>
I don't know that the file itself is generated, or at least I can't seem to figure out where its generated from, if it is. A svn co of the google mock source has it there on checkout.

> Also, I suspect it would be safer to use the more verbose:
>
> noexcept(noexcept(((MockClass*)0)->~MockClass()))
>
> on the destructor definitions. (Isn't there an easier way to say that?)
i don't really see how this is safer :)

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Now there are merge conflicts

review: Needs Fixing
Revision history for this message
Kevin DuBois (kdub) wrote :

> Now there are merge conflicts
fixed!

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> > Also, I suspect it would be safer to use the more verbose:
> >
> > noexcept(noexcept(((MockClass*)0)->~MockClass()))
> >
> > on the destructor definitions. (Isn't there an easier way to say that?)
> i don't really see how this is safer :)

Well, if MockClass has a throwing destructor then it doesn't make the NiceMock destructor noexcept.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> > Now there are merge conflicts
> fixed!

OK

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:671
http://jenkins.qa.ubuntu.com/job/mir-ci/451/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/471
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/354
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/456
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/456/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/451/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/unit-tests/client/android/test_client_surface_interpreter.cpp'
2--- tests/unit-tests/client/android/test_client_surface_interpreter.cpp 2013-04-12 16:28:32 +0000
3+++ tests/unit-tests/client/android/test_client_surface_interpreter.cpp 2013-04-19 15:35:29 +0000
4@@ -91,7 +91,7 @@
5 surf_params.pixel_format = mir_pixel_format_abgr_8888;
6
7 mock_client_buffer = std::make_shared<NiceMock<MockClientBuffer>>();
8- mock_sync = std::make_shared<MockSyncFence>();
9+ mock_sync = std::make_shared<NiceMock<MockSyncFence>>();
10 }
11
12 MirSurfaceParameters surf_params;
13@@ -102,7 +102,7 @@
14 TEST_F(AndroidInterpreterTest, native_window_dequeue_calls_surface_get_current)
15 {
16 using namespace testing;
17- MockMirSurface mock_surface{surf_params};
18+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
19 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
20
21 EXPECT_CALL(mock_surface, get_current_buffer())
22@@ -119,7 +119,7 @@
23 ANativeWindowBuffer buffer;
24 buffer.handle = &handle;
25
26- MockMirSurface mock_surface{surf_params};
27+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
28 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
29
30 EXPECT_CALL(*mock_client_buffer, get_native_handle())
31@@ -138,7 +138,7 @@
32 using namespace testing;
33 ANativeWindowBuffer buffer;
34
35- MockMirSurface mock_surface{surf_params};
36+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
37 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
38
39 EXPECT_CALL(mock_surface, next_buffer(_,_))
40@@ -152,7 +152,7 @@
41 using namespace testing;
42 ANativeWindowBuffer buffer;
43
44- MockMirSurface mock_surface{surf_params};
45+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
46 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
47
48 EXPECT_CALL(*mock_sync, wait())
49@@ -165,7 +165,7 @@
50 TEST_F(AndroidInterpreterTest, native_window_perform_remembers_format)
51 {
52 int format = 945;
53- MockMirSurface mock_surface{surf_params};
54+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
55 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
56
57 interpreter.dispatch_driver_request_format(format);
58@@ -176,7 +176,7 @@
59
60 TEST_F(AndroidInterpreterTest, native_window_hint_query_hook)
61 {
62- MockMirSurface mock_surface{surf_params};
63+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
64 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
65 /* transform hint is a bitmask of a few options for rotation/flipping buffer. a value
66 of zero is no transform */
67@@ -188,7 +188,7 @@
68
69 TEST_F(AndroidInterpreterTest, native_window_default_width_query_hook)
70 {
71- MockMirSurface mock_surface{surf_params};
72+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
73 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
74
75 auto default_width = interpreter.driver_requests_info(NATIVE_WINDOW_DEFAULT_WIDTH);
76@@ -198,7 +198,7 @@
77
78 TEST_F(AndroidInterpreterTest, native_window_default_height_query_hook)
79 {
80- MockMirSurface mock_surface{surf_params};
81+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
82 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
83
84 auto default_height = interpreter.driver_requests_info(NATIVE_WINDOW_DEFAULT_HEIGHT);
85@@ -208,7 +208,7 @@
86
87 TEST_F(AndroidInterpreterTest, native_window_width_query_hook)
88 {
89- MockMirSurface mock_surface{surf_params};
90+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
91 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
92
93 auto width = interpreter.driver_requests_info(NATIVE_WINDOW_WIDTH);
94@@ -218,7 +218,7 @@
95
96 TEST_F(AndroidInterpreterTest, native_window_height_query_hook)
97 {
98- MockMirSurface mock_surface{surf_params};
99+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
100 mcla::ClientSurfaceInterpreter interpreter(mock_surface);
101
102 auto height = interpreter.driver_requests_info(NATIVE_WINDOW_HEIGHT);
103
104=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
105--- tests/unit-tests/client/test_client_mir_surface.cpp 2013-04-09 08:50:40 +0000
106+++ tests/unit-tests/client/test_client_mir_surface.cpp 2013-04-19 15:35:29 +0000
107@@ -464,7 +464,7 @@
108 using namespace ::testing;
109
110 auto mock_input_platform = std::make_shared<mt::MockClientInputPlatform>();
111- auto mock_input_thread = std::make_shared<mt::MockInputReceiverThread>();
112+ auto mock_input_thread = std::make_shared<NiceMock<mt::MockInputReceiverThread>>();
113 MirEventDelegate delegate = {null_event_callback, nullptr};
114
115 EXPECT_CALL(*mock_buffer_factory, create_buffer(_,_,_)).Times(2);
116
117=== modified file 'tests/unit-tests/graphics/android/test_android_display_factory.cpp'
118--- tests/unit-tests/graphics/android/test_android_display_factory.cpp 2013-04-12 16:28:32 +0000
119+++ tests/unit-tests/graphics/android/test_android_display_factory.cpp 2013-04-19 15:35:29 +0000
120@@ -73,9 +73,9 @@
121 {
122 public:
123 AndroidDisplayFactoryTest()
124- : mock_display_allocator(std::make_shared<MockDisplayAllocator>()),
125- mock_hwc_factory(std::make_shared<MockHWCFactory>()),
126- mock_fnw_factory(std::make_shared<MockFNWFactory>())
127+ : mock_display_allocator(std::make_shared<testing::NiceMock<MockDisplayAllocator>>()),
128+ mock_hwc_factory(std::make_shared<testing::NiceMock<MockHWCFactory>>()),
129+ mock_fnw_factory(std::make_shared<testing::NiceMock<MockFNWFactory>>())
130 {
131 }
132
133@@ -87,7 +87,7 @@
134 std::shared_ptr<MockDisplayAllocator> const mock_display_allocator;
135 std::shared_ptr<MockHWCFactory> const mock_hwc_factory;
136 std::shared_ptr<MockFNWFactory> const mock_fnw_factory;
137- mt::HardwareAccessMock hw_access_mock;
138+ testing::NiceMock<mt::HardwareAccessMock> hw_access_mock;
139 };
140 }
141
142
143=== modified file 'tests/unit-tests/graphics/android/test_framebuffer_factory.cpp'
144--- tests/unit-tests/graphics/android/test_framebuffer_factory.cpp 2013-04-12 16:28:32 +0000
145+++ tests/unit-tests/graphics/android/test_framebuffer_factory.cpp 2013-04-19 15:35:29 +0000
146@@ -47,7 +47,7 @@
147 public:
148 FBFactory()
149 : mock_buffer_allocator(std::make_shared<testing::NiceMock<MockAndroidGraphicBufferAllocator>>()),
150- mock_display_info_provider(std::make_shared<mtd::MockDisplaySupportProvider>()),
151+ mock_display_info_provider(std::make_shared<testing::NiceMock<mtd::MockDisplaySupportProvider>>()),
152 fake_fb_num(2)
153 {
154 using namespace testing;
155
156=== modified file 'tests/unit-tests/graphics/android/test_hwc_device.cpp'
157--- tests/unit-tests/graphics/android/test_hwc_device.cpp 2013-04-16 23:27:11 +0000
158+++ tests/unit-tests/graphics/android/test_hwc_device.cpp 2013-04-19 15:35:29 +0000
159@@ -62,8 +62,8 @@
160 virtual void SetUp()
161 {
162 mock_device = std::make_shared<testing::NiceMock<mtd::MockHWCComposerDevice1>>();
163- mock_fbdev = std::make_shared<MockFBDevice>();
164- mock_organizer = std::make_shared<MockHWCOrganizer>();
165+ mock_fbdev = std::make_shared<testing::NiceMock<MockFBDevice>>();
166+ mock_organizer = std::make_shared<testing::NiceMock<MockHWCOrganizer>>();
167 }
168
169 std::shared_ptr<MockHWCOrganizer> mock_organizer;
170
171=== modified file 'tests/unit-tests/graphics/android/test_server_interpreter.cpp'
172--- tests/unit-tests/graphics/android/test_server_interpreter.cpp 2013-04-12 16:28:32 +0000
173+++ tests/unit-tests/graphics/android/test_server_interpreter.cpp 2013-04-19 15:35:29 +0000
174@@ -54,8 +54,8 @@
175 mock_buffer1 = std::make_shared<NiceMock<mtd::MockAndroidBuffer>>();
176 mock_buffer2 = std::make_shared<NiceMock<mtd::MockAndroidBuffer>>();
177 mock_buffer3 = std::make_shared<NiceMock<mtd::MockAndroidBuffer>>();
178- mock_swapper = std::make_shared<MockFBSwapper>();
179- mock_display_poster = std::make_shared<mtd::MockDisplaySupportProvider>();
180+ mock_swapper = std::make_shared<NiceMock<MockFBSwapper>>();
181+ mock_display_poster = std::make_shared<NiceMock<mtd::MockDisplaySupportProvider>>();
182 ON_CALL(*mock_display_poster, display_format())
183 .WillByDefault(Return(geom::PixelFormat::abgr_8888));
184 stub_sync = std::make_shared<StubFence>();
185
186=== modified file 'tests/unit-tests/graphics/egl_mock/hw_mock.cpp'
187--- tests/unit-tests/graphics/egl_mock/hw_mock.cpp 2013-03-29 22:52:10 +0000
188+++ tests/unit-tests/graphics/egl_mock/hw_mock.cpp 2013-04-19 15:35:29 +0000
189@@ -77,10 +77,10 @@
190 assert(global_hw_mock == NULL && "Only one mock object per process is allowed");
191 global_hw_mock = this;
192
193- mock_alloc_device = std::make_shared<mtd::MockAllocDevice>();
194+ mock_alloc_device = std::make_shared<NiceMock<mtd::MockAllocDevice>>();
195 mock_gralloc_module = std::make_shared<mt::HardwareModuleStub>(mock_alloc_device->common);
196
197- mock_hwc_device = std::make_shared<mtd::MockHWCComposerDevice1>();
198+ mock_hwc_device = std::make_shared<NiceMock<mtd::MockHWCComposerDevice1>>();
199 mock_hwc_module = std::make_shared<mt::HardwareModuleStub>(mock_hwc_device->common);
200
201 ON_CALL(*this, hw_get_module(StrEq(GRALLOC_HARDWARE_MODULE_ID),_))
202
203=== modified file 'tests/unit-tests/graphics/test_graphics_platform.cpp'
204--- tests/unit-tests/graphics/test_graphics_platform.cpp 2013-04-11 10:14:37 +0000
205+++ tests/unit-tests/graphics/test_graphics_platform.cpp 2013-04-19 15:35:29 +0000
206@@ -88,7 +88,7 @@
207 ::testing::NiceMock<mir::EglMock> mock_egl;
208 ::testing::NiceMock<mir::GLMock> mock_gl;
209 #ifdef ANDROID
210- mir::test::HardwareAccessMock hw_access_mock;
211+ ::testing::NiceMock<mir::test::HardwareAccessMock> hw_access_mock;
212 #else
213 ::testing::NiceMock<mg::gbm::MockDRM> mock_drm;
214 ::testing::NiceMock<mg::gbm::MockGBM> mock_gbm;
215
216=== modified file 'tests/unit-tests/shell/test_application_session.cpp'
217--- tests/unit-tests/shell/test_application_session.cpp 2013-04-01 16:11:03 +0000
218+++ tests/unit-tests/shell/test_application_session.cpp 2013-04-19 15:35:29 +0000
219@@ -68,11 +68,11 @@
220 {
221 InSequence seq;
222 EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
223- .WillOnce(Return(std::make_shared<mtd::MockSurface>(mt::fake_shared(surface_builder))));
224- EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
225- .WillOnce(Return(std::make_shared<mtd::MockSurface>(mt::fake_shared(surface_builder))));
226- EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
227- .WillOnce(Return(std::make_shared<mtd::MockSurface>(mt::fake_shared(surface_builder))));
228+ .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
229+ EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
230+ .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
231+ EXPECT_CALL(surface_factory, create_surface(_)).Times(1)
232+ .WillOnce(Return(std::make_shared<NiceMock<mtd::MockSurface>>(mt::fake_shared(surface_builder))));
233 }
234
235 msh::ApplicationSession app_session(mt::fake_shared(surface_factory), "Foo");
236
237=== modified file 'tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp'
238--- tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp 2013-04-18 22:04:49 +0000
239+++ tests/unit-tests/shell/test_single_visibility_focus_mechanism.cpp 2013-04-19 15:35:29 +0000
240@@ -66,7 +66,7 @@
241
242 NiceMock<mtd::MockInputFocusSelector> input_focus_selector;
243
244- MockShellSession app1, app2, app3;
245+ NiceMock<MockShellSession> app1, app2, app3;
246 msh::DefaultSessionContainer model;
247
248 ON_CALL(app1, default_surface()).WillByDefault(Return(std::shared_ptr<msh::Surface>()));
249@@ -96,7 +96,7 @@
250
251 mtd::MockInputFocusSelector input_focus_selector;
252 msh::DefaultSessionContainer model;
253- auto session = std::make_shared<MockShellSession>();
254+ auto session = std::make_shared<NiceMock<MockShellSession>>();
255 auto surface = std::make_shared<mtd::MockSurface>(std::make_shared<mtd::StubSurfaceBuilder>());
256
257 msh::SingleVisibilityFocusMechanism focus_mechanism(mt::fake_shared(model), mt::fake_shared(input_focus_selector));

Subscribers

People subscribed via source and target branches