I wonder if we should move the error trap system into xorg-gtest. <div class="gmail_extra"> <div class="gmail_quote">On Tue, Nov 6, 2012 at 10:57 PM, Peter Hutterer <<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Xlib's default error handler prints the error and calls exit(1). Tests that accidentally trigger an error thus quit without cleaning up properly. Install a default error handler that prints the basic info and continue with the test. Clients that expect to trigger errors should set a custom error handler. Signed-off-by: Peter Hutterer <<a href="mailto:peter.hutterer@who-t.net">peter.hutterer@who-t.net</a>> --- include/xorg/gtest/xorg-gtest-xserver.h | 13 ++++++ src/xserver.cpp | 54 +++++++++++++++++++++++- test/xserver-test.cpp | 73 +++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h index 8bf7996..11fc93d 100644 --- a/include/xorg/gtest/xorg-gtest-xserver.h +++ b/include/xorg/gtest/xorg-gtest-xserver.h @@ -265,6 +265,19 @@ class XServer : public xorg::testing::Process { */ static void RegisterXIOErrorHandler(); + /** + * Install a default XErrorHandler. That error handler will cause a test + * failure if called. + * + * This function is called automatically by XServer::Start(). Usually, + * you will not need to call this function unless your test does not + * instantiate and Start() an XServer object. + * + * This function will only install a new error handler if the currently + * installed XErrorHandler is not the default handler used by Xlib. + */ + static void RegisterXErrorHandler(); + private: struct Private; std::auto_ptr<Private> d_; diff --git a/src/xserver.cpp b/src/xserver.cpp index ad018a1..4faa8e9 100644 --- a/src/xserver.cpp +++ b/src/xserver.cpp @@ -394,6 +394,40 @@ const std::string& xorg::testing::XServer::GetVersion(void) { return d_->version; } +static int _x_error_handler(Display *dpy, XErrorEvent *err) +{ + std::stringstream error; + switch(err->error_code) { + case BadRequest: error << "BadRequest"; break; + case BadValue: error << "BadValue"; break; + case BadWindow: error << "BadWindow"; break; + case BadPixmap: error << "BadPixmap"; break; + case BadAtom: error << "BadAtom"; break; + case BadCursor: error << "BadCursor"; break; + case BadFont: error << "BadFont"; break; + case BadMatch: error << "BadMatch"; break; + case BadDrawable: error << "BadDrawable"; break; + case BadAccess: error << "BadAccess"; break; + case BadAlloc: error << "BadAlloc"; break; + case BadColor: error << "BadColor"; break; + case BadGC: error << "BadGC"; break; + case BadIDChoice: error << "BadIDChoice"; break; + case BadName: error << "BadName"; break; + case BadLength: error << "BadLength"; break; + case BadImplementation: error << "BadImplementation"; break; + default: + error << err->error_code; + break; + } + + ADD_FAILURE() << "XError received: " << error.str() << ", request " << + (int)err->request_code << "(" << (int)err->minor_code << "), detail: " + << err->resourceid << "\nThis error handler is likely to be triggered " + "more than once.\nCheck the first error for the real error"; + return 0; +} + + static int _x_io_error_handler(Display *dpy) _X_NORETURN; static int _x_io_error_handler(Display *dpy) { @@ -409,6 +443,15 @@ void xorg::testing::XServer::RegisterXIOErrorHandler() XSetIOErrorHandler(old_handler); } +void xorg::testing::XServer::RegisterXErrorHandler() +{ + XErrorHandler old_handler; + old_handler = XSetErrorHandler(_x_error_handler); + + if (old_handler != _XDefaultError) + XSetErrorHandler(old_handler); +} + void xorg::testing::XServer::Start(const std::string &program) { TestStartup(); @@ -464,7 +507,15 @@ void xorg::testing::XServer::Start(const std::string &program) { args.push_back(it->second); } - Process::Start(program.empty() ? d_->path_to_server : program, args); + std::string server_binary = program.empty() ? d_->path_to_server : program; + + if (getenv("XORG_GTEST_XSERVER_USE_VALGRIND")) { + args.insert(args.begin(), server_binary); + server_binary = "valgrind"; + args.insert(args.begin(), "--leak-check=full"); + } + + Process::Start(server_binary, args); /* noreturn */ } @@ -494,6 +545,7 @@ void xorg::testing::XServer::Start(const std::string &program) { signal(SIGUSR1 ,SIG_IGN); RegisterXIOErrorHandler(); + RegisterXErrorHandler(); } bool xorg::testing::XServer::Terminate(unsigned int timeout) { diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp index cdf0bd9..32792e6 100644 --- a/test/xserver-test.cpp +++ b/test/xserver-test.cpp @@ -6,6 +6,7 @@ #include <stdexcept> #include <xorg/gtest/xorg-gtest.h> +#include <gtest/gtest-spi.h> #include <X11/extensions/XInput2.h> using namespace xorg::testing; @@ -213,6 +214,78 @@ TEST(XServer, IOErrorException) }, XIOError); } +TEST(XServer, ErrorHandler) +{ + XORG_TESTCASE("Start a server, trigger a BadColor error and expect a " + "failure from the default error handler\n"); + + pid_t pid = fork(); + + if (pid == 0) { + EXPECT_NONFATAL_FAILURE({ + XServer server; + server.SetOption("-logfile", LOGFILE_DIR "/xorg-error-handler-test.log"); + server.SetOption("-config", DUMMY_CONF_PATH); + server.SetOption("-noreset", ""); + server.Start(); + ASSERT_EQ(server.GetState(), Process::RUNNING); + ::Display *dpy = XOpenDisplay(server.GetDisplayString().c_str()); + ASSERT_TRUE(dpy != NULL); + XColor color; + XQueryColor(dpy, 0, &color); + XSync(dpy, False); + }, "BadColor"); + exit(0); + } + + /* if the Xlib default error handler triggers, child calls exit(1) */ + int status; + ASSERT_EQ(waitpid(pid, &status, 0), pid); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(WEXITSTATUS(status), 0); +} + +static bool error_handler_triggered = false; + +static int _test_error_handler(Display *dpy, XErrorEvent *error) { + error_handler_triggered = true; + if (error->error_code != BadColor) + ADD_FAILURE() << "Error handler triggered with wrong error code\n"; + return 0; +} + +TEST(XServer, NondefaultErrorHandler) +{ + XORG_TESTCASE("Set a custom error handler, start a server, trigger a " + "BadColor error and expect the custom error handler to be " + "triggered\n"); + + pid_t pid = fork(); + + if (pid == 0) { + XSetErrorHandler(_test_error_handler); + + XServer server; + server.SetOption("-logfile", LOGFILE_DIR "/xorg-error-handler-test.log"); + server.SetOption("-config", DUMMY_CONF_PATH); + server.SetOption("-noreset", ""); + server.Start(); + ASSERT_EQ(server.GetState(), Process::RUNNING); + ::Display *dpy = XOpenDisplay(server.GetDisplayString().c_str()); + ASSERT_TRUE(dpy != NULL); + XColor color; + XQueryColor(dpy, 0, &color); + XSync(dpy, False); + exit(error_handler_triggered ? 0 : 1); + } + + /* if the Xlib default error handler triggers, child calls exit(1) */ + int status; + ASSERT_EQ(waitpid(pid, &status, 0), pid); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(WEXITSTATUS(status), 0); +} + TEST(XServer, KeepAlive) { XORG_TESTCASE("If XORG_GTEST_XSERVER_KEEPALIVE is set,\n" -- 1.7.11.7 _______________________________________________ <a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development Archives: <a href="http://lists.x.org/archives/xorg-devel" target="_blank">http://lists.x.org/archives/xorg-devel</a> Info: <a href="http://lists.x.org/mailman/listinfo/xorg-devel" target="_blank">http://lists.x.org/mailman/listinfo/xorg-devel</a> </blockquote></div> -- Jasper </div>