Re: [PATCH xorg-gtest 2/2] xserver: install default X error handler
On Thu, Nov 08, 2012 at 05:46:12PM -0800, Chase Douglas wrote:
> On Tue, Nov 6, 2012 at 7:57 PM, Peter Hutterer
> 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
> > ---
> > 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 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");
> > +}
> >
>
> While this looks totally cool, it probably belongs in a separate commit :).
oh, that's where that hunk ended up :) I was wondering which branch it was
on, and could not find the git commit referring to it. got committed by
accident.
> Without this hunk:
>
> R
Re: [PATCH xorg-gtest 2/2] xserver: install default X error handler
On Tue, Nov 6, 2012 at 7:57 PM, Peter Hutterer 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
> ---
> 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 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");
> +}
>
While this looks totally cool, it probably belongs in a separate commit :).
Without this hunk:
Reviewed-by: Chase Douglas
You can also add my Reviewed-by tag to a separate commit for just this
hunk. You might want to consider allowing for some standard options like
--show-reachable
-- Chase
___
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
Re: [PATCH xorg-gtest 2/2] xserver: install default X error handler
On Wed, Nov 07, 2012 at 12:56:53AM -0500, Jasper St. Pierre wrote:
> 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"
first, bad nomenclature on my side here - "clients" == tests written against
xorg-gtest.
The basic idea however is that xorg-gtest is a framework that makes starting
X servers and associates easy and manageable. anything test-specific goes
elsewhere. Now, whether the error trapping code is provided here or the
tests implement their own is up for discussion, but the current code we have
in XIT is unsuitable. Unless we trap once per test (in which case trapping
again in the test will cause an error) we cannot use it as-is.
This patch however integrates with the current trapping code: for explicit
trapping we can use SetErrorTrap/ReleaseErrorTrap, outside of that it falls
back to the default error handler provided here. Which is really just a "do
not exit(1) and break all my tests" (all tests are in the same process,
so if the default handler calls exit(1) we don't just terminate the current
test but all future ones, and we don't generate useful logs)
Cheers,
Peter
>
>
> On Wed, Nov 7, 2012 at 12:55 AM, Peter Hutterer
> 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.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
> > > > ---
> > > > 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 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: err
Re: [PATCH xorg-gtest 2/2] xserver: install default X error handler
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 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.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
> > > ---
> > > 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 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_h
Re: [PATCH xorg-gtest 2/2] xserver: install default X error handler
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
> 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
> > ---
> > 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 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(), serve
Re: [PATCH xorg-gtest 2/2] xserver: install default X error handler
I wonder if we should move the error trap system into xorg-gtest.
On Tue, Nov 6, 2012 at 10:57 PM, Peter Hutterer 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
> ---
> 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 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 @@
> #i
[PATCH xorg-gtest 2/2] xserver: install default X error handler
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
---
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 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
#include
+#include
#include
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_NO
