On Tue, Nov 6, 2012 at 7:57 PM, Peter Hutterer <[email protected]>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 <[email protected]> > --- > 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"); > + } > While this looks totally cool, it probably belongs in a separate commit :). Without this hunk: Reviewed-by: Chase Douglas <[email protected]> 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
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
