Merge lp:~vanvugt/mir/fix-StaleFrames-test-race into lp:mir
- fix-StaleFrames-test-race
- Merge into development-branch
Status: | Superseded |
---|---|
Proposed branch: | lp:~vanvugt/mir/fix-StaleFrames-test-race |
Merge into: | lp:mir |
Diff against target: |
211 lines (+117/-3) 1 file modified
tests/integration-tests/test_stale_frames.cpp (+117/-3) |
To merge this branch: | bzr merge lp:~vanvugt/mir/fix-StaleFrames-test-race |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mir CI Bot | continuous-integration | Needs Fixing | |
Mir development team | Pending | ||
Review via email: mp+313751@code.launchpad.net |
Commit message
Fix a race in the StaleFrames integration tests and expand the tests
to cover both swap intervals (since zero exploits the problem most
easily).
Description of the change
Mir CI Bot (mir-ci-bot) wrote : | # |
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3903
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3904
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3905
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3906
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3907
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'tests/integration-tests/test_stale_frames.cpp' |
2 | --- tests/integration-tests/test_stale_frames.cpp 2017-01-09 06:48:47 +0000 |
3 | +++ tests/integration-tests/test_stale_frames.cpp 2017-01-11 11:51:00 +0000 |
4 | @@ -20,6 +20,9 @@ |
5 | #include "mir_toolkit/debug/surface.h" |
6 | |
7 | #include "mir/compositor/compositor.h" |
8 | +#include "mir/scene/surface.h" |
9 | +#include "mir/scene/surface_factory.h" |
10 | +#include "mir/scene/null_surface_observer.h" |
11 | #include "mir/renderer/renderer_factory.h" |
12 | #include "mir/graphics/renderable.h" |
13 | #include "mir/graphics/buffer.h" |
14 | @@ -36,6 +39,7 @@ |
15 | #include <mutex> |
16 | #include <condition_variable> |
17 | |
18 | +using namespace std::chrono_literals; |
19 | namespace mtf = mir_test_framework; |
20 | namespace mtd = mir::test::doubles; |
21 | namespace mc = mir::compositor; |
22 | @@ -114,8 +118,66 @@ |
23 | StubRenderer* renderer_ = nullptr; |
24 | }; |
25 | |
26 | +class PostObserver : public mir::scene::NullSurfaceObserver |
27 | +{ |
28 | +public: |
29 | + PostObserver(std::function<void(int)> cb) : cb{cb} |
30 | + { |
31 | + } |
32 | + |
33 | + void frame_posted(int count, geom::Size const&) override |
34 | + { |
35 | + cb(count); |
36 | + } |
37 | + |
38 | +private: |
39 | + std::function<void(int)> const cb; |
40 | +}; |
41 | + |
42 | +class PublicSurfaceFactory : public mir::scene::SurfaceFactory |
43 | +{ |
44 | +public: |
45 | + using Surface = mir::scene::Surface; |
46 | + |
47 | + PublicSurfaceFactory(std::shared_ptr<mir::scene::SurfaceFactory> const& real) : real_surface_factory{real} {} |
48 | + |
49 | + std::shared_ptr<Surface> create_surface( |
50 | + std::list<mir::scene::StreamInfo> const& streams, |
51 | + mir::scene::SurfaceCreationParameters const& params) override |
52 | + { |
53 | + latest_surface = real_surface_factory->create_surface(streams, params); |
54 | + return latest_surface; |
55 | + } |
56 | + |
57 | + std::shared_ptr<Surface> const latest() const |
58 | + { |
59 | + return latest_surface; |
60 | + } |
61 | + |
62 | + void forget_latest() |
63 | + { |
64 | + latest_surface.reset(); |
65 | + } |
66 | + |
67 | +private: |
68 | + std::shared_ptr<mir::scene::SurfaceFactory> const real_surface_factory; |
69 | + std::shared_ptr<Surface> latest_surface; |
70 | +}; |
71 | + |
72 | struct StubServerConfig : mtf::StubbedServerConfiguration |
73 | { |
74 | + std::shared_ptr<PublicSurfaceFactory> the_public_surface_factory() |
75 | + { |
76 | + return public_surface_factory( |
77 | + [this]{ return std::make_shared<PublicSurfaceFactory>( |
78 | + mtf::StubbedServerConfiguration::the_surface_factory()); }); |
79 | + } |
80 | + |
81 | + std::shared_ptr<mir::scene::SurfaceFactory> the_surface_factory() override |
82 | + { |
83 | + return the_public_surface_factory(); |
84 | + } |
85 | + |
86 | std::shared_ptr<StubRendererFactory> the_stub_renderer_factory() |
87 | { |
88 | return stub_renderer_factory( |
89 | @@ -128,17 +190,30 @@ |
90 | } |
91 | |
92 | mir::CachedPtr<StubRendererFactory> stub_renderer_factory; |
93 | + mir::CachedPtr<PublicSurfaceFactory> public_surface_factory; |
94 | }; |
95 | |
96 | using BasicFixture = mtf::BasicClientServerFixture<StubServerConfig>; |
97 | |
98 | -struct StaleFrames : BasicFixture |
99 | +struct StaleFrames : BasicFixture, |
100 | + ::testing::WithParamInterface<int> |
101 | { |
102 | + StaleFrames() |
103 | + : post_observer(std::make_shared<PostObserver>( |
104 | + [this](int n){frame_posted(n);})) |
105 | + { |
106 | + } |
107 | + |
108 | void SetUp() |
109 | { |
110 | BasicFixture::SetUp(); |
111 | |
112 | client_create_surface(); |
113 | + auto pub = server_configuration.the_public_surface_factory(); |
114 | + auto surface = pub->latest(); |
115 | + ASSERT_TRUE(surface); |
116 | + surface->add_observer(post_observer); |
117 | + pub->forget_latest(); |
118 | } |
119 | |
120 | void TearDown() |
121 | @@ -170,12 +245,45 @@ |
122 | server_configuration.the_compositor()->start(); |
123 | } |
124 | |
125 | + /* |
126 | + * NOTE that we wait for surface buffer posts as opposed to display posts. |
127 | + * The difference is that surface buffer posts will precisely match the |
128 | + * number of client swaps for any swap interval, but display posts may be |
129 | + * fewer than the number of swaps if the client was quick and using |
130 | + * interval zero. |
131 | + */ |
132 | + void frame_posted(int count) |
133 | + { |
134 | + std::unique_lock<std::mutex> lock(mutex); |
135 | + posts += count; |
136 | + posted.notify_all(); |
137 | + } |
138 | + |
139 | + bool wait_for_posts(int count, std::chrono::seconds timeout) |
140 | + { |
141 | + std::unique_lock<std::mutex> lock(mutex); |
142 | + auto const deadline = std::chrono::steady_clock::now() + timeout; |
143 | + while (posts < count) |
144 | + { |
145 | + if (posted.wait_until(lock, deadline) == std::cv_status::timeout) |
146 | + return false; |
147 | + } |
148 | + posts -= count; |
149 | + return true; |
150 | + } |
151 | + |
152 | MirWindow* window; |
153 | + |
154 | +private: |
155 | + std::shared_ptr<PostObserver> post_observer; |
156 | + std::mutex mutex; |
157 | + std::condition_variable posted; |
158 | + int posts = 0; |
159 | }; |
160 | |
161 | } |
162 | |
163 | -TEST_F(StaleFrames, are_dropped_when_restarting_compositor) |
164 | +TEST_P(StaleFrames, are_dropped_when_restarting_compositor) |
165 | { |
166 | using namespace testing; |
167 | |
168 | @@ -186,6 +294,7 @@ |
169 | stale_buffers.emplace(mir_debug_surface_current_buffer_id(window)); |
170 | |
171 | auto bs = mir_window_get_buffer_stream(window); |
172 | + mir_buffer_stream_set_swapinterval(bs, GetParam()); |
173 | mir_buffer_stream_swap_buffers_sync(bs); |
174 | |
175 | stale_buffers.emplace(mir_debug_surface_current_buffer_id(window)); |
176 | @@ -196,6 +305,7 @@ |
177 | auto const fresh_buffer = mg::BufferID{mir_debug_surface_current_buffer_id(window)}; |
178 | mir_buffer_stream_swap_buffers_sync(bs); |
179 | |
180 | + ASSERT_TRUE(wait_for_posts(3, 60s)); |
181 | start_compositor(); |
182 | |
183 | // Note first stale buffer and fresh_buffer may be equal when defaulting to double buffers |
184 | @@ -206,22 +316,26 @@ |
185 | EXPECT_THAT(stale_buffers, Not(Contains(new_buffers[0]))); |
186 | } |
187 | |
188 | -TEST_F(StaleFrames, only_fresh_frames_are_used_after_restarting_compositor) |
189 | +TEST_P(StaleFrames, only_fresh_frames_are_used_after_restarting_compositor) |
190 | { |
191 | using namespace testing; |
192 | |
193 | stop_compositor(); |
194 | |
195 | auto bs = mir_window_get_buffer_stream(window); |
196 | + mir_buffer_stream_set_swapinterval(bs, GetParam()); |
197 | mir_buffer_stream_swap_buffers_sync(bs); |
198 | mir_buffer_stream_swap_buffers_sync(bs); |
199 | |
200 | auto const fresh_buffer = mg::BufferID{mir_debug_surface_current_buffer_id(window)}; |
201 | mir_buffer_stream_swap_buffers_sync(bs); |
202 | |
203 | + ASSERT_TRUE(wait_for_posts(3, 60s)); |
204 | start_compositor(); |
205 | |
206 | auto const new_buffers = wait_for_new_rendered_buffers(); |
207 | ASSERT_THAT(new_buffers.size(), Eq(1u)); |
208 | EXPECT_THAT(new_buffers[0], Eq(fresh_buffer)); |
209 | } |
210 | + |
211 | +INSTANTIATE_TEST_CASE_P(PerSwapInterval, StaleFrames, ::testing::Values(0,1)); |
PASSED: Continuous integration, rev:3902 /mir-jenkins. ubuntu. com/job/ mir-ci/ 2479/ /mir-jenkins. ubuntu. com/job/ build-mir/ 3233 /mir-jenkins. ubuntu. com/job/ build-0- fetch/3300 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 3292 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial+ overlay/ 3292 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= yakkety/ 3292 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= yakkety/ 3262 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= yakkety/ 3262/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3262 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3262/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= yakkety/ 3262 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= yakkety/ 3262/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 3262 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 3262/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 3262 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 3262/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3262 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3262/artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /mir-jenkins. ubuntu. com/job/ mir-ci/ 2479/rebuild
https:/