I wonder if we should move the error trap system into xorg-gtest.
On Tue, Nov 6, 2012 at 10:57 PM, Peter Hutterer <peter.hutte...@who-t.net>wrote:
> 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 <peter.hutte...@who-t.net>
> ---
> 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
>
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
--
Jasper
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
