Mir

Merge lp:~vanvugt/mir/fix-1342092 into lp:mir

fix-1342092
Merge into development-branch

Proposed by Daniel van Vugt on 2016-02-26

Status:

Superseded

Proposed branch:

lp:~vanvugt/mir/fix-1342092

Merge into:

lp:mir

Diff against target:

361 lines (+109/-25)

9 files modified

include/test/mir_test_framework/interprocess_client_server_test.h (+2/-0)
tests/acceptance-tests/server_signal_handling.cpp (+8/-2)
tests/acceptance-tests/test_server_shutdown.cpp (+5/-5)
tests/acceptance-tests/throwback/test_client_library_errors.cpp (+14/-6)
tests/include/mir/test/death.h (+49/-0)
tests/mir_test_framework/interprocess_client_server_test.cpp (+12/-0)
tests/unit-tests/dispatch/test_threaded_dispatcher.cpp (+7/-3)
tests/unit-tests/test_fatal.cpp (+7/-5)
tests/unit-tests/test_udev_wrapper.cpp (+5/-4)

To merge this branch:

bzr merge lp:~vanvugt/mir/fix-1342092

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Kevin DuBois (community)		2016-03-14	Needs Fixing on 2016-03-17
Mir CI Bot	continuous-integration		Needs Fixing on 2016-03-17
Alan Griffiths			Needs Fixing on 2016-03-17
Chris Halse Rogers			Needs Information on 2016-03-01
Alexandros Frantzis (community)		2016-02-26	Needs Fixing on 2016-02-26
Review via email: mp+287271@code.launchpad.net

Commit message

Avoid dumping core from death tests that are expected to die
(LP: #1342092)

But try to remember to only disable cores in child processes, so that
faulty tests can still dump core.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-26:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/412/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2016-02-26:

+ ${PROJECT_SOURCE_DIR}/tests/include # Use for mir/test/death.h only

We should publish death.h in include/test/... to keep the acceptance tests from using any internal code.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-02-26:

+ ${PROJECT_SOURCE_DIR}/tests/include # Use for mir/test/death.h only

We intentionally don't have this directory in the path to avoid accidental use of unpublished APIs in the acceptance tests.

~~~~

- EXPECT_DEATH(mtf::make_any_surface(connection), "");
+ EXPECT_DEATH(
+ {
+ mt::disable_core_dump();
+ mtf::make_any_surface(connection);
+ }, "");

This is rather an intrusive approach as "mt::disable_core_dump();" isn't really part of the test.
Could we instead introduce:

EXPECT_DEATH_OMIT_CORE(mtf::make_any_surface(connection), "");

~~~~

- run_in_server([&]{ kill(getpid(), SIGABRT); });
+ run_in_server([&]
+ {
+ mt::disable_core_dump();
+ kill(getpid(), SIGABRT);
+ });

This is rather an intrusive approach as "mt::disable_core_dump();" isn't really part of the test.
Could we instead introduce:

run_in_server_omit_core([&]{ kill(getpid(), SIGABRT); });

~~~~

+#include <sys/resource.h>
+
+namespace mir { namespace test {
+
+inline void disable_core_dump()

We don't really need to add <sys/resource.h> to a bunch of tests. This would probably be better out of line.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-29:

Alf:
Yeah I don't mind too much. Although I'm disappointed we need to publish headers, publicly, just to maintain consistency. Otherwise there's no good reason to publish it. Also, I did find a bunch of include/test headers like the mtf already published. Those are most naturally internal to Mir, and only published to support QtMir's use of internal Mir classes I would guess. So it's not a new problem but I would prefer to not be forced to make it worse.

Alan:
Yeah a new macro might help. Although I partially disagree with run_in_server_omit_core. It does reduce lines of code, but does not change the number of new functions. And I feel your "omit" syntax is less readable than what I have already proposed.

As for resource.h, that's not a strong argument. It's a C system header so the impact of that will be negligible compared to all the C++ template fluff we use.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-03-01:

Wouldn't this be more easily done by changing how we run our DeathTests (ie: running them under a core file ulimit log)? We already segregate out all of our tests into *DeathTests and run them differently.

Relatedly, are all of these changes necessary? Specifically, the run_in_server ones shouldn't be dumping core, right? They'll be hitting the our signal handler and triggering an orderly shutdown. That's what those tests are testing :).

review: Needs Information

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-03-01:

Both good points. However...

Running DeathTests in a special way with ulimit -c 0 would prevent those tests from dumping core if and when the test was faulty (ie. something we don't expect to crash has crashed). And we do want a core file in that case. Also doing that would be a scripting hack instead of a nice code change.

And I would have agreed that catching fatal signals should prevent them from dumping core. However that's apparently not true here. Perhaps we re-raise them as SIGABRT and that's the problem? But regardless, you probably don't want to go changing core logic when the goal here is to just change the tests.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-03-02:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/468/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-03-07:

+ typedef enum { disable_core_dump = 1 } RunFlag;
+ typedef unsigned int RunFlags;

This is C++, please use "enum class"

~~~~

+ void run_in_server(std::function<void()> const& exec_code,
+ RunFlags flags = 0);

https://unity.ubuntu.com/mir/cppguide/index.html?showone=Default_Arguments#Default_Arguments

~~~~

+ EXPECT_DEATH( \
...
+ EXPECT_EXIT( \

It seems a little odd that these we rely on user code to include a header that defines these.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-03-14:

I think you're raising silly issues, which is now counter-productive as the MP has stagnated for a week.

They're really unimportant and I disagree with them. But being unimportant, I don't care to make this MP last even longer with arguments.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-03-14:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/552/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-03-15:

Addressing Alan's concerns:

enum class: I tried, but then discovered 'enum class' removes implicit int conversion required to use the enum values as bit flags. So the code doesn't compile if you use 'enum class'.

Default arguments: I feel the current syntax is a bit nicer than adding a new function with some longer name. It also scales better into the future if new RunFlags are added.

Missing include: Yes, it was intentionally odd as an exercise in include-minimalism. But fixed now.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-03-15:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/569/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-03-15:

@"Fix most of Alan's concerns. But don't use enum class because gcc then
refuses to accept the & operator. And not using default parameters would
necessitate a change in function name, which I feel would not aid in
maintainability at all."

We have a single option - "core on" or "core off". Why does that require implicit conversion to an int and bit operators?

Not using default parameters does not /require/ a change to the function name. In C++ functions can be overloaded.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-03-16:

You raise good points, but let me explain a bit better:

enum class: It's not intended to be a singleton. It's intended to be one of multiple bit flags in future. As such 'enum class' does not work.

Default parameters: Indeed, I could just add another function of the same name. However if you look at the cppguide (that bit authored by Google?), then the argument against default parameters only makes sense if each function has a different name. If each function has the same name then that has the same problems as the cppguide cites as "Cons".

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-03-16:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/591/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-03-16:

> You raise good points, but let me explain a bit better:
>
> enum class: It's not intended to be a singleton. It's intended to be one of
> multiple bit flags in future. As such 'enum class' does not work.

YAGNI

> Default parameters: Indeed, I could just add another function of the same
> name. However if you look at the cppguide (that bit authored by Google?), then
> the argument against default parameters only makes sense if each function has
> a different name. If each function has the same name then that has the same
> problems as the cppguide cites as "Cons".

The guide says "We do not allow default function parameter..." this isn't one of the exceptions.

Following the guide promotes consistency in the codebase and we shouldn't ignore it on a whim - even where we might personally disagree. There's quite a bit of the guide I don't agree with, but the team voted to follow it and I try to follow it. In the really painful areas I've MP'd changes and some of those were accepted.

In this case, providing

void mtf::InterprocessClientServerTest::run_in_server(std::function<void()> const& exec_code)
{
run_in_server(exec_code, 0);
}

seems pretty painless.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-03-17:

This is again counter-productive.

YAGNI is not a sensible answer here. The penalty for being flexible is one extra typedef.

And blindly following the guide when I've just explained how you misinterpreted the guide is incorrect. We should clarify the guide, or simply apply common sense and not follow it blindly when clearly there is relevant information missing that would change the outcome.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-03-17:

I not am misinterpreting the guide.

You wish me to approve code that departs from the guide in two ways. One you justify with "flexibility" that isn't needed, the other because you disagree with the explanation given for the rule.

I don't think either departure is justified by those arguments.

FWIW If I were writing the code both of these points would be moot as I'd publish two functions with different names and not have a "flags" argument in the interface. If it would end this silly discussion I'd propose that version.

review: Needs Fixing

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-03-17:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/608/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Kevin DuBois (kdub) wrote on 2016-03-17:

re the guide and default arguments,
Seems we should stick to the guide. We have changed the guide, and can keep doing so if a bit of it doesn't make sense. The bit about default arguments make sense to me as it is in the guide (perhaps though, I wouldn't use default arguments for giving the impression of variable length parameters)

re the enum class,
Seems pretty painless to change?

re Adding a new function:
Having a second function that will run the code in the server without the core dump seems to give the user (especially one that's building off existing code without reading the header first) the most information about what the function call will do.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Daniel van Vugt

Emanuele Antonio Faraone

Gerry Boland

Mir development team

1	=== modified file 'include/test/mir_test_framework/interprocess_client_server_test.h'
2	--- include/test/mir_test_framework/interprocess_client_server_test.h 2016-01-29 08:18:22 +0000
3	+++ include/test/mir_test_framework/interprocess_client_server_test.h 2016-03-18 04:20:17 +0000
4	@@ -37,6 +37,8 @@
5	void init_server(std::function<void()> const& init_code);
6
7	void run_in_server(std::function<void()> const& exec_code);
8	+ void run_in_server_and_disable_core_dump(
9	+ std::function<void()> const& exec_code);
10
11	void run_in_client(std::function<void()> const& client_code);
12
13
14	=== modified file 'tests/acceptance-tests/server_signal_handling.cpp'
15	--- tests/acceptance-tests/server_signal_handling.cpp 2016-01-29 08:18:22 +0000
16	+++ tests/acceptance-tests/server_signal_handling.cpp 2016-03-18 04:20:17 +0000
17	@@ -82,7 +82,10 @@
18	{
19	expect_server_signalled(GetParam());
20
21	- run_in_server([&]{ kill(getpid(), GetParam()); });
22	+ run_in_server_and_disable_core_dump([&]
23	+ {
24	+ kill(getpid(), GetParam());
25	+ });
26
27	cleanup_done.wait_for_signal_ready_for(timeout);
28	}
29	@@ -105,7 +108,10 @@
30	server.add_emergency_cleanup([&] { cleanup.signal_ready(); });
31	});
32
33	- run_in_server([&]{ kill(getpid(), SIGABRT); });
34	+ run_in_server_and_disable_core_dump([&]
35	+ {
36	+ kill(getpid(), SIGABRT);
37	+ });
38
39	cleanup_done.wait_for_signal_ready_for(timeout);
40	for (auto& cleanup : more_cleanup)
41
42	=== modified file 'tests/acceptance-tests/test_server_shutdown.cpp'
43	--- tests/acceptance-tests/test_server_shutdown.cpp 2016-01-29 08:18:22 +0000
44	+++ tests/acceptance-tests/test_server_shutdown.cpp 2016-03-18 04:20:17 +0000
45	@@ -76,7 +76,7 @@
46	{
47	mt::CrossProcessSync sync;
48
49	- run_in_server([&]
50	+ run_in_server_and_disable_core_dump([&]
51	{
52	sync.wait_for_signal_ready_for();
53	abort();
54	@@ -106,7 +106,7 @@
55
56	mt::CrossProcessSync sync;
57
58	- run_in_server([&]
59	+ run_in_server_and_disable_core_dump([&]
60	{
61	sync.wait_for_signal_ready_for();
62	mir::fatal_error("Bang");
63	@@ -131,7 +131,7 @@
64
65	mt::CrossProcessSync sync;
66
67	- run_in_server([&]
68	+ run_in_server_and_disable_core_dump([&]
69	{
70	sync.wait_for_signal_ready_for();
71	mir::fatal_error("Bang");
72	@@ -155,7 +155,7 @@
73
74	mt::CrossProcessSync sync;
75
76	- run_in_server([&]
77	+ run_in_server_and_disable_core_dump([&]
78	{
79	sync.wait_for_signal_ready_for();
80	mir::fatal_error("Bang");
81	@@ -207,7 +207,7 @@
82	{
83	mt::CrossProcessSync sync;
84
85	- run_in_server([&]
86	+ run_in_server_and_disable_core_dump([&]
87	{
88	sync.wait_for_signal_ready_for();
89	raise(GetParam());
90
91	=== modified file 'tests/acceptance-tests/throwback/test_client_library_errors.cpp'
92	--- tests/acceptance-tests/throwback/test_client_library_errors.cpp 2016-03-16 06:59:01 +0000
93	+++ tests/acceptance-tests/throwback/test_client_library_errors.cpp 2016-03-18 04:20:17 +0000
94	@@ -24,6 +24,7 @@
95	#include "src/include/client/mir/client_buffer_factory.h"
96
97	#include "mir/test/validity_matchers.h"
98	+#include "mir/test/death.h"
99
100	#include "mir_test_framework/headless_in_process_server.h"
101	#include "mir_test_framework/using_client_platform.h"
102	@@ -308,8 +309,7 @@
103	auto connection = mir_connect_sync("garbage", __PRETTY_FUNCTION__);
104
105	ASSERT_FALSE(mir_connection_is_valid(connection));
106	- EXPECT_DEATH(
107	- mtf::make_any_surface(connection), "");
108	+ MIR_EXPECT_DEATH(mtf::make_any_surface(connection), "");
109
110	mir_connection_release(connection);
111	}
112	@@ -322,7 +322,7 @@
113	auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
114
115	ASSERT_FALSE(mir_connection_is_valid(connection));
116	- EXPECT_DEATH(mtf::make_any_surface(connection), "");
117	+ MIR_EXPECT_DEATH(mtf::make_any_surface(connection), "");
118
119	mir_connection_release(connection);
120	}
121	@@ -334,7 +334,7 @@
122	auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
123
124	ASSERT_FALSE(mir_connection_is_valid(connection));
125	- EXPECT_DEATH(mtf::make_any_surface(connection), "");
126	+ MIR_EXPECT_DEATH(mtf::make_any_surface(connection), "");
127
128	mir_connection_release(connection);
129	}
130	@@ -351,7 +351,11 @@
131	10,
132	10
133	};
134	- EXPECT_DEATH(mir_surface_spec_attach_to_foreign_parent(spec, nullptr, &rect, mir_edge_attachment_any), "");
135	+ MIR_EXPECT_DEATH(
136	+ {
137	+ mir_surface_spec_attach_to_foreign_parent(spec, nullptr, &rect,
138	+ mir_edge_attachment_any);
139	+ }, "");
140
141	mir_connection_release(connection);
142	}
143	@@ -364,7 +368,11 @@
144
145	auto id = mir_persistent_id_from_string("fa69b2e9-d507-4005-be61-5068f40a5aec");
146
147	- EXPECT_DEATH(mir_surface_spec_attach_to_foreign_parent(spec, id, nullptr, mir_edge_attachment_any), "");
148	+ MIR_EXPECT_DEATH(
149	+ {
150	+ mir_surface_spec_attach_to_foreign_parent(spec, id, nullptr,
151	+ mir_edge_attachment_any);
152	+ }, "");
153
154	mir_persistent_id_release(id);
155	mir_connection_release(connection);
156
157	=== added file 'tests/include/mir/test/death.h'
158	--- tests/include/mir/test/death.h 1970-01-01 00:00:00 +0000
159	+++ tests/include/mir/test/death.h 2016-03-18 04:20:17 +0000
160	@@ -0,0 +1,49 @@
161	+/*
162	+ * Copyright © 2016 Canonical Ltd.
163	+ *
164	+ * This program is free software: you can redistribute it and/or modify it
165	+ * under the terms of the GNU General Public License version 3,
166	+ * as published by the Free Software Foundation.
167	+ *
168	+ * This program is distributed in the hope that it will be useful,
169	+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
170	+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
171	+ * GNU General Public License for more details.
172	+ *
173	+ * You should have received a copy of the GNU General Public License
174	+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
175	+ *
176	+ * Authored by: Daniel van Vugt <daniel.van.vugt@canonical.com>
177	+ */
178	+
179	+#ifndef MIR_TEST_DEATH_H_
180	+#define MIR_TEST_DEATH_H_
181	+
182	+#include <sys/resource.h>
183	+#include <gtest/gtest.h>
184	+
185	+namespace mir { namespace test {
186	+
187	+inline void disable_core_dump()
188	+{
189	+ struct rlimit zeroes{0,0};
190	+ setrlimit(RLIMIT_CORE, &zeroes);
191	+}
192	+
193	+} } // namespace mir::test
194	+
195	+#define MIR_EXPECT_DEATH(statement, message) \
196	+ EXPECT_DEATH( \
197	+ { \
198	+ ::mir::test::disable_core_dump(); \
199	+ { statement ; } \
200	+ }, message);
201	+
202	+#define MIR_EXPECT_EXIT(statement, how, message) \
203	+ EXPECT_EXIT( \
204	+ { \
205	+ ::mir::test::disable_core_dump(); \
206	+ { statement ; } \
207	+ }, how, message);
208	+
209	+#endif /* MIR_TEST_DEATH_H_ */
210
211	=== modified file 'tests/mir_test_framework/interprocess_client_server_test.cpp'
212	--- tests/mir_test_framework/interprocess_client_server_test.cpp 2016-01-29 08:18:22 +0000
213	+++ tests/mir_test_framework/interprocess_client_server_test.cpp 2016-03-18 04:20:17 +0000
214	@@ -18,9 +18,11 @@
215
216	#include "mir_test_framework/interprocess_client_server_test.h"
217	#include "mir_test_framework/process.h"
218	+#include "mir/test/death.h"
219
220	#include <gtest/gtest.h>
221	#include <gmock/gmock.h>
222	+#include <sys/resource.h>
223
224	namespace mt = mir::test;
225	namespace mtf = mir_test_framework;
226	@@ -52,6 +54,16 @@
227	}
228	}
229
230	+void mtf::InterprocessClientServerTest::run_in_server_and_disable_core_dump(
231	+ std::function<void()> const& exec_code)
232	+{
233	+ run_in_server([exec_code]()
234	+ {
235	+ mir::test::disable_core_dump();
236	+ exec_code();
237	+ });
238	+}
239	+
240	void mtf::InterprocessClientServerTest::run_in_server(std::function<void()> const& exec_code)
241	{
242	if (test_process_id != getpid()) return;
243
244	=== modified file 'tests/unit-tests/dispatch/test_threaded_dispatcher.cpp'
245	--- tests/unit-tests/dispatch/test_threaded_dispatcher.cpp 2016-03-16 06:59:01 +0000
246	+++ tests/unit-tests/dispatch/test_threaded_dispatcher.cpp 2016-03-18 04:20:17 +0000
247	@@ -19,6 +19,7 @@
248	#include "mir/dispatch/threaded_dispatcher.h"
249	#include "mir/dispatch/dispatchable.h"
250	#include "mir/fd.h"
251	+#include "mir/test/death.h"
252	#include "mir/test/pipe.h"
253	#include "mir/test/signal.h"
254	#include "mir/test/test_dispatchable.h"
255	@@ -272,8 +273,11 @@
256	// Ideally we'd use terminate_with_current_exception, but that's in server
257	// and we're going to be called from a C context anyway, so just using the default
258	// std::terminate behaviour should be acceptable.
259	- EXPECT_EXIT({ md::ThreadedDispatcher dispatcher("Die!", dispatchable); std::this_thread::sleep_for(10s); },
260	- KilledBySignal(SIGABRT), (std::string{"."} + exception_msg + ".").c_str());
261	+ MIR_EXPECT_EXIT(
262	+ {
263	+ md::ThreadedDispatcher dispatcher("Die!", dispatchable);
264	+ std::this_thread::sleep_for(10s);
265	+ }, KilledBySignal(SIGABRT), (std::string{"."} + exception_msg + ".").c_str());
266	}
267
268	TEST_F(ThreadedDispatcherTest, sets_thread_names_appropriately)
269	@@ -387,7 +391,7 @@
270	using namespace testing;
271	using namespace std::literals::chrono_literals;
272
273	- EXPECT_EXIT(
274	+ MIR_EXPECT_EXIT(
275	{
276	md::ThreadedDispatcher* dispatcher;
277
278
279	=== modified file 'tests/unit-tests/test_fatal.cpp'
280	--- tests/unit-tests/test_fatal.cpp 2015-06-17 05:20:42 +0000
281	+++ tests/unit-tests/test_fatal.cpp 2016-03-18 04:20:17 +0000
282	@@ -18,6 +18,7 @@
283	*/
284
285	#include "mir/fatal.h"
286	+#include "mir/test/death.h"
287	#include <gtest/gtest.h>
288	#include <gmock/gmock.h>
289	#include <csignal>
290	@@ -28,17 +29,18 @@
291	{
292	mir::FatalErrorStrategy on_error{mir::fatal_error_abort};
293
294	- EXPECT_DEATH({mir::fatal_error("%s had %d %s %s", "Mary", 1, "little", "lamb");},
295	- "Mary had 1 little lamb");
296	+ MIR_EXPECT_DEATH(mir::fatal_error("%s had %d %s %s",
297	+ "Mary", 1, "little", "lamb"),
298	+ "Mary had 1 little lamb");
299	}
300
301	TEST(FatalErrorDeathTest, abort_raises_sigabrt)
302	{
303	mir::FatalErrorStrategy on_error{mir::fatal_error_abort};
304
305	- EXPECT_EXIT({mir::fatal_error("Hello world");},
306	- KilledBySignal(SIGABRT),
307	- "Hello world");
308	+ MIR_EXPECT_EXIT(mir::fatal_error("Hello world"),
309	+ KilledBySignal(SIGABRT),
310	+ "Hello world");
311	}
312
313	TEST(FatalErrorTest, throw_formats_message_to_what)
314
315	=== modified file 'tests/unit-tests/test_udev_wrapper.cpp'
316	--- tests/unit-tests/test_udev_wrapper.cpp 2016-01-29 08:18:22 +0000
317	+++ tests/unit-tests/test_udev_wrapper.cpp 2016-03-18 04:20:17 +0000
318	@@ -18,6 +18,7 @@
319
320	#include "mir_test_framework/udev_environment.h"
321	#include "mir/udev/wrapper.h"
322	+#include "mir/test/death.h"
323
324	#include <gtest/gtest.h>
325	#include <memory>
326	@@ -293,7 +294,7 @@
327
328	devices.scan_devices();
329
330	- EXPECT_EXIT((*devices.end()).subsystem(), KilledByInvalidMemoryAccess, "");
331	+ MIR_EXPECT_EXIT((*devices.end()).subsystem(), KilledByInvalidMemoryAccess, "");
332
333	auto iter = devices.begin();
334
335	@@ -301,7 +302,7 @@
336	{
337	iter++;
338	}
339	- EXPECT_EXIT((*iter).subsystem(), KilledByInvalidMemoryAccess, "");
340	+ MIR_EXPECT_EXIT((*iter).subsystem(), KilledByInvalidMemoryAccess, "");
341	}
342
343	TEST_F(UdevWrapperTest, MemberDereferenceWorks)
344	@@ -328,7 +329,7 @@
345
346	devices.scan_devices();
347
348	- EXPECT_EXIT(devices.end()->subsystem(), KilledByInvalidMemoryAccess, "");
349	+ MIR_EXPECT_EXIT(devices.end()->subsystem(), KilledByInvalidMemoryAccess, "");
350
351	auto iter = devices.begin();
352
353	@@ -336,7 +337,7 @@
354	{
355	iter++;
356	}
357	- EXPECT_EXIT(iter->subsystem(), KilledByInvalidMemoryAccess, "");
358	+ MIR_EXPECT_EXIT(iter->subsystem(), KilledByInvalidMemoryAccess, "");
359	}
360
361	TEST_F(UdevWrapperTest, UdevMonitorDoesNotTriggerBeforeEnabling)

Mir

Merge lp:~vanvugt/mir/fix-1342092 into lp:mir

Commit message

Description of the change

Preview Diff

Subscribers