Mir

Merge lp:~cemil-azizoglu/mir/fix-chatty-logging-in-tests into lp:mir

Proposed by Cemil Azizoglu
Status: Rejected
Rejected by: Cemil Azizoglu
Proposed branch: lp:~cemil-azizoglu/mir/fix-chatty-logging-in-tests
Merge into: lp:mir
Prerequisite: lp:~alan-griffiths/mir/fix-set_logger
Diff against target: 143 lines (+35/-5)
8 files modified
include/platform/mir/options/configuration.h (+1/-0)
src/common/logging/logger.cpp (+1/-2)
src/include/common/mir/logging/logger.h (+8/-0)
src/platform/options/default_configuration.cpp (+4/-2)
src/platform/symbols.map (+1/-0)
src/server/default_server_configuration.cpp (+4/-1)
tests/mir_test_framework/CMakeLists.txt (+6/-0)
tests/mir_test_framework/headless_test.cpp (+10/-0)
To merge this branch: bzr merge lp:~cemil-azizoglu/mir/fix-chatty-logging-in-tests
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Alan Griffiths Needs Fixing
Alexandros Frantzis (community) Needs Fixing
Review via email: mp+242441@code.launchpad.net

Description of the change

Fix https://bugs.launchpad.net/mir/+bug/1394221

- An option was added to send logs to NullLogger.
- Acceptance tests now use this option.

To post a comment you must log in.
2081. By Cemil Azizoglu

Whitespace

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> 3 - server-ABI-unchanged (Failed)
> 6 - platform-ABI-unchanged (Failed)

CI failure.

> 75 + (silent_logger_opt, "Send logs to a black hole.");

Clearer as "--disable-logging", "Disable logging."?

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

W: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/vivid/universe/i18n/Translation-en Hash Sum mismatch

triggering rebuild

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

*Needs discussion*

I'm not convinced this should be a mirserver option - I think logging should be "always on".

I've MP'd an alternative that only touches the test framework:

lp:~alan-griffiths/mir/fix-1394221/+merge/242477

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

131 +#if !defined(ALLOW_CONSOLE_LOGGING_IN_TESTS)
132 + const int argc = 2;
133 + char const* argv[argc] = {
134 + __PRETTY_FUNCTION__,
135 + "-s" /* --silent-logger */
136 + };
137 +
138 + server.set_command_line(argc, argv);
139 +#endif

needing to recompile to operate a logging switch is not intuitive.

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

Unmerged revisions

2081. By Cemil Azizoglu

Whitespace

2080. By Cemil Azizoglu

- Option to turn send logs to a black hole
- Use the option during tests

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/platform/mir/options/configuration.h'
2--- include/platform/mir/options/configuration.h 2014-11-17 13:43:52 +0000
3+++ include/platform/mir/options/configuration.h 2014-11-21 00:12:57 +0000
4@@ -45,6 +45,7 @@
5 extern char const* const fatal_abort_opt;
6 extern char const* const debug_opt;
7 extern char const* const use_asio_main_loop_opt;
8+extern char const* const silent_logger_opt;
9
10 extern char const* const name_opt;
11 extern char const* const offscreen_opt;
12
13=== modified file 'src/common/logging/logger.cpp'
14--- src/common/logging/logger.cpp 2014-11-17 18:26:11 +0000
15+++ src/common/logging/logger.cpp 2014-11-21 00:12:57 +0000
16@@ -16,7 +16,6 @@
17 * Authored by: Cemil Azizoglu <cemil.azizoglu@canonical.com>
18 */
19
20-#include "mir/logging/dumb_console_logger.h"
21 #include "mir/logging/logger.h"
22
23 #include <memory>
24@@ -39,7 +38,7 @@
25 {
26 std::lock_guard<decltype(log_mutex)> lock{log_mutex};
27 if (!the_logger)
28- the_logger = std::make_shared<ml::DumbConsoleLogger>();
29+ the_logger = std::make_shared<ml::NullLogger>();
30
31 return the_logger;
32 }
33
34=== modified file 'src/include/common/mir/logging/logger.h'
35--- src/include/common/mir/logging/logger.h 2014-11-17 18:26:11 +0000
36+++ src/include/common/mir/logging/logger.h 2014-11-21 00:12:57 +0000
37@@ -52,6 +52,14 @@
38 Logger& operator=(const Logger&) = delete;
39 };
40
41+class NullLogger : public Logger
42+{
43+public:
44+ void log(Severity,
45+ const std::string&,
46+ const std::string&) override {}
47+};
48+
49 void log(Severity severity, const std::string& message);
50 void log(Severity severity, const std::string& message, const std::string& component);
51 void set_logger(std::shared_ptr<Logger> const& new_logger);
52
53=== modified file 'src/platform/options/default_configuration.cpp'
54--- src/platform/options/default_configuration.cpp 2014-11-17 13:43:52 +0000
55+++ src/platform/options/default_configuration.cpp 2014-11-21 00:12:57 +0000
56@@ -43,10 +43,11 @@
57 char const* const mo::frontend_threads_opt = "ipc-thread-pool";
58 char const* const mo::name_opt = "name";
59 char const* const mo::offscreen_opt = "offscreen";
60-char const* const mo::touchspots_opt = "enable-touchspots";
61+char const* const mo::touchspots_opt = "enable-touchspots";
62 char const* const mo::fatal_abort_opt = "on-fatal-error-abort";
63 char const* const mo::debug_opt = "debug";
64 char const* const mo::use_asio_main_loop_opt = "use-asio-main-loop";
65+char const* const mo::silent_logger_opt = "silent-logger,s";
66
67 char const* const mo::off_opt_value = "off";
68 char const* const mo::log_opt_value = "log";
69@@ -147,7 +148,8 @@
70 "in unexpected ways] abort (to get a core dump)")
71 (debug_opt, "Enable extra development debugging. "
72 "This is only interesting for people doing Mir server or client development.")
73- (use_asio_main_loop_opt, "Use the ASIO main loop implementation");
74+ (use_asio_main_loop_opt, "Use the ASIO main loop implementation")
75+ (silent_logger_opt, "Send logs to a black hole.");
76
77 add_platform_options();
78 }
79
80=== modified file 'src/platform/symbols.map'
81--- src/platform/symbols.map 2014-11-19 13:45:25 +0000
82+++ src/platform/symbols.map 2014-11-21 00:12:57 +0000
83@@ -178,6 +178,7 @@
84 mir::options::session_mediator_report_opt*;
85 mir::options::touchspots_opt*;
86 mir::options::use_asio_main_loop_opt*;
87+ mir::options::silent_logger_opt*;
88 non-virtual?thunk?to?mir::graphics::Cursor::?Cursor*;
89 non-virtual?thunk?to?mir::graphics::CursorImage::?CursorImage*;
90 non-virtual?thunk?to?mir::graphics::DisplayConfigurationPolicy::?DisplayConfigurationPolicy*;
91
92=== modified file 'src/server/default_server_configuration.cpp'
93--- src/server/default_server_configuration.cpp 2014-11-17 13:43:52 +0000
94+++ src/server/default_server_configuration.cpp 2014-11-21 00:12:57 +0000
95@@ -212,6 +212,9 @@
96 return logger(
97 [this]() -> std::shared_ptr<ml::Logger>
98 {
99- return std::make_shared<ml::DumbConsoleLogger>();
100+ if (the_options()->is_set(options::silent_logger_opt))
101+ return std::make_shared<ml::NullLogger>();
102+ else
103+ return std::make_shared<ml::DumbConsoleLogger>();
104 });
105 }
106
107=== modified file 'tests/mir_test_framework/CMakeLists.txt'
108--- tests/mir_test_framework/CMakeLists.txt 2014-11-18 16:15:00 +0000
109+++ tests/mir_test_framework/CMakeLists.txt 2014-11-21 00:12:57 +0000
110@@ -35,6 +35,12 @@
111 fake_event_hub_server_configuration.cpp
112 )
113
114+option(MIR_ALLOW_CONSOLE_LOGGING_IN_TESTS "Allow logs to stdout during tests" OFF)
115+
116+if (MIR_ALLOW_CONSOLE_LOGGING_IN_TESTS)
117+add_definitions(-DALLOW_CONSOLE_LOGGING_IN_TESTS)
118+endif (MIR_ALLOW_CONSOLE_LOGGING_IN_TESTS)
119+
120 list(APPEND TEST_FRAMEWORK_SRCS
121 socket_detect_server.cpp
122 )
123
124=== modified file 'tests/mir_test_framework/headless_test.cpp'
125--- tests/mir_test_framework/headless_test.cpp 2014-11-06 03:56:24 +0000
126+++ tests/mir_test_framework/headless_test.cpp 2014-11-21 00:12:57 +0000
127@@ -42,6 +42,16 @@
128
129 void mtf::HeadlessTest::start_server()
130 {
131+#if !defined(ALLOW_CONSOLE_LOGGING_IN_TESTS)
132+ const int argc = 2;
133+ char const* argv[argc] = {
134+ __PRETTY_FUNCTION__,
135+ "-s" /* --silent-logger */
136+ };
137+
138+ server.set_command_line(argc, argv);
139+#endif
140+
141 server.add_init_callback([&]
142 {
143 auto const main_loop = server.the_main_loop();

Subscribers

People subscribed via source and target branches