Mir

Merge lp:~vanvugt/mir/fix-1192618 into lp:~mir-team/mir/trunk

  1. fix-1192618
  2. Merge into trunk
Proposed by Daniel van Vugt
Status: Merged
Approved by: Robert Ancell
Approved revision: no longer in the source branch.
Merged at revision: 884
Proposed branch: lp:~vanvugt/mir/fix-1192618
Merge into: lp:~mir-team/mir/trunk
Diff against target: 148 lines (+29/-42)
4 files modified
include/test/mir_test_doubles/stub_buffer_allocator.h (+23/-15)
tests/integration-tests/compositor/test_buffer_stream.cpp (+2/-23)
tests/integration-tests/compositor/test_swapping_swappers.cpp (+2/-2)
tests/integration-tests/test_surfaceloop.cpp (+2/-2)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1192618
Reviewer Review Type Date Requested Status
Robert Carr (community) Approve
Alexandros Frantzis (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+176612@code.launchpad.net

Commit message

Silence noisy integration tests (Uninteresting mock function calls)
(LP: #1192618)

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

PASSED: Continuous integration, rev:882
http://jenkins.qa.ubuntu.com/job/mir-ci/1090/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1424
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/328
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/328/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/708

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

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Needing expectations in the set-up phase instead of in the actual test is a strong indication that something is not right. What's wrong in both these cases is that we don't actually need mocks, we just need stubs. Not using stubs is a pre-existing issue, but this is a good opportunity to fix it.

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

PASSED: Continuous integration, rev:884
http://jenkins.qa.ubuntu.com/job/mir-ci/1092/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1427
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1312
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/330
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/330/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/710

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

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

144 + EXPECT_CALL(*buffer_allocation_strategy, create_swapper_new_buffers(_,_,_))
145 + .WillOnce(testing::Invoke(this, &ServerConfig::on_create_swapper));

We can change this to use a stub, too, since this is not really a test expectation.

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> 144 + EXPECT_CALL(*buffer_allocation_strategy, create_swapper_new_buffers(_,_,_))
> 145 + .WillOnce(testing::Invoke(this, &ServerConfig::on_create_swapper));

> We can change this to use a stub, too, since this is not really a test expectation.

... at least according to the original test. But I guess we can consider it an expectation if we want.

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'include/test/mir_test_doubles/mock_buffer_allocator.h' => 'include/test/mir_test_doubles/stub_buffer_allocator.h'
2--- include/test/mir_test_doubles/mock_buffer_allocator.h 2013-07-23 10:07:33 +0000
3+++ include/test/mir_test_doubles/stub_buffer_allocator.h 2013-07-24 10:38:27 +0000
4@@ -13,11 +13,11 @@
5 * You should have received a copy of the GNU General Public License
6 * along with this program. If not, see <http://www.gnu.org/licenses/>.
7 *
8- * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
9+ * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
10 */
11
12-#ifndef MIR_TEST_DOUBLES_MOCK_BUFFER_ALLOCATOR_H_
13-#define MIR_TEST_DOUBLES_MOCK_BUFFER_ALLOCATOR_H_
14+#ifndef MIR_TEST_DOUBLES_STUB_BUFFER_ALLOCATOR_H_
15+#define MIR_TEST_DOUBLES_STUB_BUFFER_ALLOCATOR_H_
16
17 #include "mir/compositor/graphic_buffer_allocator.h"
18 #include "mir_test_doubles/stub_buffer.h"
19@@ -35,22 +35,30 @@
20 namespace doubles
21 {
22
23-struct MockBufferAllocator : public compositor::GraphicBufferAllocator
24+struct StubBufferAllocator : public compositor::GraphicBufferAllocator
25 {
26- MockBufferAllocator()
27- {
28- using namespace testing;
29- ON_CALL(*this, alloc_buffer(_))
30- .WillByDefault(Return(std::make_shared<StubBuffer>()));
31- }
32- ~MockBufferAllocator() noexcept{}
33-
34- MOCK_METHOD1(alloc_buffer, std::shared_ptr<graphics::Buffer>(compositor::BufferProperties const&));
35- MOCK_METHOD0(supported_pixel_formats, std::vector<geometry::PixelFormat>());
36+ StubBufferAllocator() : id{1} {}
37+
38+ std::shared_ptr<graphics::Buffer> alloc_buffer(
39+ compositor::BufferProperties const&)
40+ {
41+ compositor::BufferProperties properties{geometry::Size{id, id},
42+ geometry::PixelFormat::abgr_8888,
43+ compositor::BufferUsage::hardware};
44+ ++id;
45+ return std::make_shared<StubBuffer>(properties);
46+ }
47+
48+ std::vector<geometry::PixelFormat> supported_pixel_formats()
49+ {
50+ return {};
51+ }
52+
53+ unsigned int id;
54 };
55
56 }
57 }
58 }
59
60-#endif // MIR_TEST_DOUBLES_MOCK_BUFFER_ALLOCATOR_H_
61+#endif // MIR_TEST_DOUBLES_STUB_BUFFER_ALLOCATOR_H_
62
63=== modified file 'tests/integration-tests/compositor/test_buffer_stream.cpp'
64--- tests/integration-tests/compositor/test_buffer_stream.cpp 2013-07-18 11:04:18 +0000
65+++ tests/integration-tests/compositor/test_buffer_stream.cpp 2013-07-24 10:38:27 +0000
66@@ -22,6 +22,7 @@
67 #include "src/server/compositor/switching_bundle.h"
68
69 #include "mir_test_doubles/stub_buffer.h"
70+#include "mir_test_doubles/stub_buffer_allocator.h"
71 #include "multithread_harness.h"
72
73 #include <gmock/gmock.h>
74@@ -39,28 +40,6 @@
75 namespace
76 {
77
78-struct StubBufferAllocator : public mc::GraphicBufferAllocator
79-{
80- StubBufferAllocator() : id{1} {}
81-
82- std::shared_ptr<mg::Buffer> alloc_buffer(mc::BufferProperties const&)
83- {
84- mc::BufferProperties properties{geom::Size{id, id},
85- geom::PixelFormat::abgr_8888,
86- mc::BufferUsage::hardware};
87- ++id;
88- return std::make_shared<mtd::StubBuffer>(properties);
89- }
90-
91- std::vector<geom::PixelFormat> supported_pixel_formats()
92- {
93- return {};
94- }
95-
96- unsigned int id;
97-};
98-
99-
100 struct BufferStreamTest : public ::testing::Test
101 {
102 BufferStreamTest()
103@@ -70,7 +49,7 @@
104
105 std::shared_ptr<mc::BufferBundle> create_bundle()
106 {
107- auto allocator = std::make_shared<StubBufferAllocator>();
108+ auto allocator = std::make_shared<mtd::StubBufferAllocator>();
109 auto factory = std::make_shared<mc::SwapperFactory>(allocator);
110 mc::BufferProperties properties{geom::Size{380, 210},
111 geom::PixelFormat::abgr_8888,
112
113=== modified file 'tests/integration-tests/compositor/test_swapping_swappers.cpp'
114--- tests/integration-tests/compositor/test_swapping_swappers.cpp 2013-07-23 10:07:33 +0000
115+++ tests/integration-tests/compositor/test_swapping_swappers.cpp 2013-07-24 10:38:27 +0000
116@@ -16,7 +16,7 @@
117 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
118 */
119
120-#include "mir_test_doubles/mock_buffer_allocator.h"
121+#include "mir_test_doubles/stub_buffer_allocator.h"
122 #include "multithread_harness.h"
123
124 #include "src/server/compositor/switching_bundle.h"
125@@ -44,7 +44,7 @@
126 {
127 void SetUp()
128 {
129- auto allocator = std::make_shared<mtd::MockBufferAllocator>();
130+ auto allocator = std::make_shared<mtd::StubBufferAllocator>();
131 auto factory = std::make_shared<mc::SwapperFactory>(allocator, 3);
132 auto properties = mc::BufferProperties{geom::Size{380, 210},
133 geom::PixelFormat::abgr_8888, mc::BufferUsage::hardware};
134
135=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
136--- tests/integration-tests/test_surfaceloop.cpp 2013-07-19 18:15:01 +0000
137+++ tests/integration-tests/test_surfaceloop.cpp 2013-07-24 10:38:27 +0000
138@@ -230,8 +230,8 @@
139 using testing::_;
140 buffer_allocation_strategy = std::make_shared<mtd::MockSwapperFactory>();
141
142- ON_CALL(*buffer_allocation_strategy, create_swapper_new_buffers(_,_,_))
143- .WillByDefault(testing::Invoke(this, &ServerConfig::on_create_swapper));
144+ EXPECT_CALL(*buffer_allocation_strategy, create_swapper_new_buffers(_,_,_))
145+ .WillOnce(testing::Invoke(this, &ServerConfig::on_create_swapper));
146 }
147 return buffer_allocation_strategy;
148 }

Subscribers

People subscribed via source and target branches