Mir

Merge lp:~vanvugt/mir/fix-1192618 into lp:~mir-team/mir/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 :
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 :
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
=== renamed file 'include/test/mir_test_doubles/mock_buffer_allocator.h' => 'include/test/mir_test_doubles/stub_buffer_allocator.h'
--- include/test/mir_test_doubles/mock_buffer_allocator.h 2013-07-23 10:07:33 +0000
+++ include/test/mir_test_doubles/stub_buffer_allocator.h 2013-07-24 10:38:27 +0000
@@ -13,11 +13,11 @@
13 * You should have received a copy of the GNU General Public License13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *15 *
16 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>16 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
17 */17 */
1818
19#ifndef MIR_TEST_DOUBLES_MOCK_BUFFER_ALLOCATOR_H_19#ifndef MIR_TEST_DOUBLES_STUB_BUFFER_ALLOCATOR_H_
20#define MIR_TEST_DOUBLES_MOCK_BUFFER_ALLOCATOR_H_20#define MIR_TEST_DOUBLES_STUB_BUFFER_ALLOCATOR_H_
2121
22#include "mir/compositor/graphic_buffer_allocator.h"22#include "mir/compositor/graphic_buffer_allocator.h"
23#include "mir_test_doubles/stub_buffer.h"23#include "mir_test_doubles/stub_buffer.h"
@@ -35,22 +35,30 @@
35namespace doubles35namespace doubles
36{36{
3737
38struct MockBufferAllocator : public compositor::GraphicBufferAllocator38struct StubBufferAllocator : public compositor::GraphicBufferAllocator
39{39{
40 MockBufferAllocator()40 StubBufferAllocator() : id{1} {}
41 {41
42 using namespace testing;42 std::shared_ptr<graphics::Buffer> alloc_buffer(
43 ON_CALL(*this, alloc_buffer(_))43 compositor::BufferProperties const&)
44 .WillByDefault(Return(std::make_shared<StubBuffer>()));44 {
45 }45 compositor::BufferProperties properties{geometry::Size{id, id},
46 ~MockBufferAllocator() noexcept{}46 geometry::PixelFormat::abgr_8888,
4747 compositor::BufferUsage::hardware};
48 MOCK_METHOD1(alloc_buffer, std::shared_ptr<graphics::Buffer>(compositor::BufferProperties const&));48 ++id;
49 MOCK_METHOD0(supported_pixel_formats, std::vector<geometry::PixelFormat>());49 return std::make_shared<StubBuffer>(properties);
50 }
51
52 std::vector<geometry::PixelFormat> supported_pixel_formats()
53 {
54 return {};
55 }
56
57 unsigned int id;
50};58};
5159
52}60}
53}61}
54}62}
5563
56#endif // MIR_TEST_DOUBLES_MOCK_BUFFER_ALLOCATOR_H_64#endif // MIR_TEST_DOUBLES_STUB_BUFFER_ALLOCATOR_H_
5765
=== modified file 'tests/integration-tests/compositor/test_buffer_stream.cpp'
--- tests/integration-tests/compositor/test_buffer_stream.cpp 2013-07-18 11:04:18 +0000
+++ tests/integration-tests/compositor/test_buffer_stream.cpp 2013-07-24 10:38:27 +0000
@@ -22,6 +22,7 @@
22#include "src/server/compositor/switching_bundle.h"22#include "src/server/compositor/switching_bundle.h"
2323
24#include "mir_test_doubles/stub_buffer.h"24#include "mir_test_doubles/stub_buffer.h"
25#include "mir_test_doubles/stub_buffer_allocator.h"
25#include "multithread_harness.h"26#include "multithread_harness.h"
2627
27#include <gmock/gmock.h>28#include <gmock/gmock.h>
@@ -39,28 +40,6 @@
39namespace40namespace
40{41{
4142
42struct StubBufferAllocator : public mc::GraphicBufferAllocator
43{
44 StubBufferAllocator() : id{1} {}
45
46 std::shared_ptr<mg::Buffer> alloc_buffer(mc::BufferProperties const&)
47 {
48 mc::BufferProperties properties{geom::Size{id, id},
49 geom::PixelFormat::abgr_8888,
50 mc::BufferUsage::hardware};
51 ++id;
52 return std::make_shared<mtd::StubBuffer>(properties);
53 }
54
55 std::vector<geom::PixelFormat> supported_pixel_formats()
56 {
57 return {};
58 }
59
60 unsigned int id;
61};
62
63
64struct BufferStreamTest : public ::testing::Test43struct BufferStreamTest : public ::testing::Test
65{44{
66 BufferStreamTest()45 BufferStreamTest()
@@ -70,7 +49,7 @@
7049
71 std::shared_ptr<mc::BufferBundle> create_bundle()50 std::shared_ptr<mc::BufferBundle> create_bundle()
72 {51 {
73 auto allocator = std::make_shared<StubBufferAllocator>();52 auto allocator = std::make_shared<mtd::StubBufferAllocator>();
74 auto factory = std::make_shared<mc::SwapperFactory>(allocator);53 auto factory = std::make_shared<mc::SwapperFactory>(allocator);
75 mc::BufferProperties properties{geom::Size{380, 210},54 mc::BufferProperties properties{geom::Size{380, 210},
76 geom::PixelFormat::abgr_8888,55 geom::PixelFormat::abgr_8888,
7756
=== modified file 'tests/integration-tests/compositor/test_swapping_swappers.cpp'
--- tests/integration-tests/compositor/test_swapping_swappers.cpp 2013-07-23 10:07:33 +0000
+++ tests/integration-tests/compositor/test_swapping_swappers.cpp 2013-07-24 10:38:27 +0000
@@ -16,7 +16,7 @@
16 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>16 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
17 */17 */
1818
19#include "mir_test_doubles/mock_buffer_allocator.h"19#include "mir_test_doubles/stub_buffer_allocator.h"
20#include "multithread_harness.h"20#include "multithread_harness.h"
2121
22#include "src/server/compositor/switching_bundle.h"22#include "src/server/compositor/switching_bundle.h"
@@ -44,7 +44,7 @@
44{44{
45 void SetUp()45 void SetUp()
46 {46 {
47 auto allocator = std::make_shared<mtd::MockBufferAllocator>();47 auto allocator = std::make_shared<mtd::StubBufferAllocator>();
48 auto factory = std::make_shared<mc::SwapperFactory>(allocator, 3);48 auto factory = std::make_shared<mc::SwapperFactory>(allocator, 3);
49 auto properties = mc::BufferProperties{geom::Size{380, 210},49 auto properties = mc::BufferProperties{geom::Size{380, 210},
50 geom::PixelFormat::abgr_8888, mc::BufferUsage::hardware};50 geom::PixelFormat::abgr_8888, mc::BufferUsage::hardware};
5151
=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
--- tests/integration-tests/test_surfaceloop.cpp 2013-07-19 18:15:01 +0000
+++ tests/integration-tests/test_surfaceloop.cpp 2013-07-24 10:38:27 +0000
@@ -230,8 +230,8 @@
230 using testing::_;230 using testing::_;
231 buffer_allocation_strategy = std::make_shared<mtd::MockSwapperFactory>();231 buffer_allocation_strategy = std::make_shared<mtd::MockSwapperFactory>();
232232
233 ON_CALL(*buffer_allocation_strategy, create_swapper_new_buffers(_,_,_))233 EXPECT_CALL(*buffer_allocation_strategy, create_swapper_new_buffers(_,_,_))
234 .WillByDefault(testing::Invoke(this, &ServerConfig::on_create_swapper));234 .WillOnce(testing::Invoke(this, &ServerConfig::on_create_swapper));
235 }235 }
236 return buffer_allocation_strategy;236 return buffer_allocation_strategy;
237 }237 }

Subscribers

People subscribed via source and target branches