[PATCH xorg-gtest 2/2] xserver: install default X error handler
Jasper St. Pierre
jstpierre at mecheye.net
Tue Nov 6 21:56:53 PST 2012
Right, but it seems to have one error handling mechanism in xorg-gtest, and
another in xorg-integration-tests, especially with your commit message of
"other clients will have to install their own error handlers"
On Wed, Nov 7, 2012 at 12:55 AM, Peter Hutterer <peter.hutterer at who-t.net>wrote:
> On Wed, Nov 07, 2012 at 12:36:16AM -0500, Jasper St. Pierre wrote:
> > I wonder if we should move the error trap system into xorg-gtest.
>
> yes, it would be useful have a unified error system for (both?) xorg-gtest
> users.
>
> this is largely orthogonal to this commit though. the error trap system is
> useful to trap specific errors that are expected, but not general errors
> that may occur accidentally - which is what this patch addresses.
>
> Cheers,
> Peter
>
> > On Tue, Nov 6, 2012 at 10:57 PM, Peter Hutterer <
> peter.hutterer at 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.hutterer at 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 at lists.x.org: X.Org development
> > > Archives: http://lists.x.org/archives/xorg-devel
> > > Info: http://lists.x.org/mailman/listinfo/xorg-devel
> > >
> >
> >
> >
> > --
> > Jasper
>
--
Jasper
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20121107/57ffdb0e/attachment.html>
More information about the xorg-devel
mailing list