On Thu, Nov 08, 2012 at 05:46:12PM -0800, Chase Douglas wrote: > 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 :).
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: > > 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 yeah, it's not complete yet, that's why I hadn't intended to send this one out just yet :) Thanks for the reviews Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
