On Mon, Apr 20, 2009 at 6:09 PM, Peter Hutterer <[email protected]> wrote: > I looked at the glib testing framework and I think it's good for what we need. > It provides a simple setup and supports more complicated use-cases (that need > seperate setup and cleanup). > It gives XML/HTML output through gtester and gtester-report, which is useful > for the tinderbox. > > Also, as airlied pointed out, glib is likely to be present anyway on a > developer's machine - an advantage over other test suites. > > Here's the updated patch, I'm quite happy with it and I'm rather eager to > merge this soon. We can fix up the automake pain at a later point in time. > > Differences to the previous patch: > - configure supports --disable-unit-tests > - glib-based instead of macro hacks > - one automake warning cleaned up (AM_LDFLAGS was misused) > - automake cleans temporary files. > > The test currently runs on "make check" only. At a later point in time we > might want to run this on each make, but currently the objcopy recreates the > library each time, and that drags down compilation time.
Seems pretty nice, but here's just a few comments on the automake/make. > > Cheers, > Peter > > From cfcc187cd879049194e8778cfee896dcdd374895 Mon Sep 17 00:00:00 2001 > From: Peter Hutterer <[email protected]> > Date: Wed, 15 Apr 2009 17:21:08 +1000 > Subject: [PATCH 1/2] Add a test-suite for in-server unit-testing. > > This patch adds a test/ directory that contains the setup for a unit-testing > suite designed for in-server unit-testing. All functions available to the X > server are available to the test binaries through static linking. > > This test suite uses the glib testing framework. > Do not use glib calls outside of the test/ directory. > > Signed-off-by: Peter Hutterer <[email protected]> > --- > configure.ac | 15 +++++ > test/Makefile.am | 52 ++++++++++++++++ > test/README | 36 +++++++++++ > test/xkb.c | 173 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 276 insertions(+), 0 deletions(-) > create mode 100644 test/Makefile.am > create mode 100644 test/README > create mode 100644 test/xkb.c > > diff --git a/configure.ac b/configure.ac > index ef50627..56847da 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -79,6 +79,9 @@ AC_SYS_LARGEFILE > XORG_PROG_RAWCPP > AC_PATH_PROG(SED,sed) > > +# for the test suite > +AC_CHECK_TOOL(OBJCOPY, objcopy) > + > dnl Check for dtrace program (needed to build Xserver dtrace probes) > dnl Also checks for <sys/sdt.h>, since some Linux distros have an > dnl ISDN trace program named dtrace > @@ -439,6 +442,9 @@ AC_ARG_ENABLE(werror, > AS_HELP_STRING([--enable-werror], > AC_ARG_ENABLE(debug, AS_HELP_STRING([--enable-debug], > [Enable debugging (default: disabled)]), > [DEBUGGING=$enableval], [DEBUGGING=no]) > +AC_ARG_ENABLE(unit-tests, AS_HELP_STRING([--enable-unit-tests], > + [Enable unit-tests (default: enabled)]), > + [UNITTESTS=$enableval], [UNITTESTS=yes]) > AC_ARG_WITH(int10, AS_HELP_STRING([--with-int10=BACKEND], [int10 > backend: vm86, x86emu or stub]), > [INT10="$withval"], > [INT10="$DEFAULT_INT10"]) > @@ -1141,6 +1147,14 @@ if test "x$DEBUGGING" = xyes; then > fi > AM_CONDITIONAL(DEBUG, [test "x$DEBUGGING" = xyes]) > > +if test "x$UNITTESTS" = xyes; then > + AC_DEFINE(UNITTESTS, 1, [Enable unit tests]) > + PKG_CHECK_MODULES([GLIB], [glib-2.0]) > + AC_SUBST([GLIB_LIBS]) > + AC_SUBST([GLIB_CFLAGS]) > +fi > +AM_CONDITIONAL(UNITTESTS, [test "x$UNITTESTS" = xyes]) > + > AC_DEFINE(XTEST, 1, [Support XTest extension]) > AC_DEFINE(XSYNC, 1, [Support XSync extension]) > AC_DEFINE(XCMISC, 1, [Support XCMisc extension]) > @@ -1976,5 +1990,6 @@ hw/kdrive/fbdev/Makefile > hw/kdrive/linux/Makefile > hw/kdrive/sdl/Makefile > hw/kdrive/src/Makefile > +test/Makefile > xorg-server.pc > ]) > diff --git a/test/Makefile.am b/test/Makefile.am > new file mode 100644 > index 0000000..6abcc92 > --- /dev/null > +++ b/test/Makefile.am > @@ -0,0 +1,52 @@ > +if UNITTESTS > +check_PROGRAMS = xkb > +check_LTLIBRARIES = libxservertest.la libxservertest_t.la > + > +TESTS=$(check_PROGRAMS) > + > +# This makefile is not happy if you have concurrent makes going on. > +AM_MAKEFLAGS = -j1 > + > +AM_CFLAGS = $(DIX_CFLAGS) $(GLIB_CFLAGS) @XORG_CFLAGS@ > +INCLUDES = @XORG_INCS@ > +TEST_LDFLAGS=.libs/libxservertest_t.a $(XORG_SYS_LIBS) $(XSERVER_SYS_LIBS) > $(GLIB_LIBS) > + > +xkb_LDFLAGS=$(TEST_LDFLAGS) It might not ever matter, but this breaks how arguments are ordered for libtool. For programs, you want to use _LDADD to add libraries. > + > + > +libxservertest_la_LIBADD = \ > + $(XSERVER_LIBS) \ > + $(top_builddir)/hw/xfree86/loader/libloader.la \ > + $(top_builddir)/hw/xfree86/os-support/libxorgos.la \ > + $(top_builddir)/hw/xfree86/common/libcommon.la \ > + $(top_builddir)/hw/xfree86/parser/libxf86config.la \ > + $(top_builddir)/hw/xfree86/dixmods/libdixmods.la \ > + $(top_builddir)/hw/xfree86/modes/libxf86modes.la \ > + $(top_builddir)/hw/xfree86/ramdac/libramdac.la \ > + $(top_builddir)/hw/xfree86/ddc/libddc.la \ > + $(top_builddir)/hw/xfree86/i2c/libi2c.la \ > + $(top_builddir)/hw/xfree86/dixmods/libxorgxkb.la \ > + $(top_builddir)/hw/xfree86/libxorg.la \ > + $(top_builddir)/mi/libmi.la \ > + $(top_builddir)/os/libos.la \ > + �...@xorg_libs@ > + > +CLEANFILES=libxservertest_t.c libxservertest.c > + > +libxservertest_t.c: > + touch $@ > + > +libxservertest.c: > + touch $@ > + > +# libxservertest.la defines main, so we need to get rid of it somehow. > +# we do this by copying libxservertest.a into libxservertest_t.a and > removing the > +# symbol during the copy process. > +libxservertest_t.la: libxservertest.la > + $(OBJCOPY) --strip-symbol=main .libs/libxservertest.a > .libs/libxservertest_t.a If this is how you want to do it, don't define libxservertest_t as a libtool library (check_LTLIBRARIES). It just confuses the tools. Just make the target libxservertest.a and then the file make is looking for (libxserverttest_t.a) will actually be there. Since you're defining a custom rule for generating the .a, there's no benefit to defining it as libtool library. It also means you don't have to create libxservertest_t.c. On the other hand, it's pretty gross. :) The two alternatives that come to mind: 1. Don't actually define main in dix. Instead have it called dixmain or something so that any DDX (or test program) can decide to override it or not. I think you already mentioned this. 2. Try to get libtool to not export main from libxservert.la. I haven't tried this, but I believe you could do: libxservertest_la_LDFLAGS = -export-symbols-regex '...' Unfortunately, my regex foo does not match my autotools foo, so I don't know a regex that says "everything except main". > + > + > +all: > + echo "Run 'make check' to run the test suite" @echo ... to suppress the command being printed. > + > +endif > diff --git a/test/README b/test/README > new file mode 100644 > index 0000000..5759a72 > --- /dev/null > +++ b/test/README > @@ -0,0 +1,36 @@ > + X server test suite > + > +This suite contains a set of tests to verify the behaviour of functions used > +internally to the server. This test suite is based on glib's testing > +framework [1]. > + > += How it works = > +Through some automake abuse, we link the test programs with the same static > +libraries as the Xorg binary. The test suites can then call various functions > +and verify their behaviour - without the need to start the server or connect > +clients. > + > +This testing only works for functions that do not rely on a particular state > +of the X server. Unless the test suite replicates the expected state, which > +may be difficult. > + > += How to run the tests = > +Run "make check" the test directory. This will compile the tests and execute > +them in the order specified in the TESTS variable in test/Makefile.am. > + > +Each set of tests related to a subsystem are available as a binary that can > be > +executed directly. For example, run "xkb" to perform some xkb-related tests. > + > +== Adding a new test == > +When adding a new test, ensure that you add a short description of what the > +test does and what the expected outcome is. If the test reproduces a > +particular bug, using g_test_bug(). > + > +== Misc == > + > +The programs "gtester" and "gtester-report" may be used to generate XML/HTML > +log files of tests succeeded and failed. > + > +--------- > + > +[1] http://library.gnome.org/devel/glib/stable/glib-Testing.html > diff --git a/test/xkb.c b/test/xkb.c > new file mode 100644 > index 0000000..6fbb26a > --- /dev/null > +++ b/test/xkb.c > @@ -0,0 +1,173 @@ > +/** > + * Copyright © 2009 Red Hat, Inc. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS > OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + */ > + > +#ifdef HAVE_DIX_CONFIG_H > +#include <dix-config.h> > +#endif > + > +#include <xkb-config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <ctype.h> > +#include <unistd.h> > +#include <math.h> > +#include <X11/X.h> > +#include <X11/Xproto.h> > +#include <X11/keysym.h> > +#include <X11/Xatom.h> > +#include "misc.h" > +#include "inputstr.h" > +#include "opaque.h" > +#include "property.h" > +#define XKBSRV_NEED_FILE_FUNCS > +#include <xkbsrv.h> > +#include "../xkb/xkbgeom.h" > +#include <X11/extensions/XKMformat.h> > +#include "xkbfile.h" > +#include "../xkb/xkb.h" > + > +#include <glib.h> > + > +/** > + * Initialize an empty XkbRMLVOSet. > + * Call XkbGetRulesDflts to obtain the default ruleset. > + * Compare obtained ruleset with the built-in defaults. > + * > + * Result: RMLVO defaults are the same as obtained. > + */ > +static void xkb_get_rules_test(void) > +{ > + XkbRMLVOSet rmlvo = { NULL}; > + XkbGetRulesDflts(&rmlvo); > + > + > + g_assert(rmlvo.rules); > + g_assert(rmlvo.model); > + g_assert(rmlvo.layout); > + g_assert(rmlvo.variant); > + g_assert(rmlvo.options); > + g_assert(strcmp(rmlvo.rules, XKB_DFLT_RULES) == 0); > + g_assert(strcmp(rmlvo.model, XKB_DFLT_MODEL) == 0); > + g_assert(strcmp(rmlvo.layout, XKB_DFLT_LAYOUT) == 0); > + g_assert(strcmp(rmlvo.variant, XKB_DFLT_VARIANT) == 0); > + g_assert(strcmp(rmlvo.options, XKB_DFLT_OPTIONS) == 0); > +} > + > +/** > + * Initialize an random XkbRMLVOSet. > + * Call XkbGetRulesDflts to obtain the default ruleset. > + * Compare obtained ruleset with the built-in defaults. > + * Result: RMLVO defaults are the same as obtained. > + */ > +static void xkb_set_rules_test(void) > +{ > + XkbRMLVOSet rmlvo = { > + .rules = "test-rules", > + .model = "test-model", > + .layout = "test-layout", > + .variant = "test-variant", > + .options = "test-options" > + }; > + XkbRMLVOSet rmlvo_new = { NULL }; > + > + XkbSetRulesDflts(&rmlvo); > + XkbGetRulesDflts(&rmlvo_new); > + > + /* XkbGetRulesDflts strdups the values */ > + g_assert(rmlvo.rules != rmlvo_new.rules); > + g_assert(rmlvo.model != rmlvo_new.model); > + g_assert(rmlvo.layout != rmlvo_new.layout); > + g_assert(rmlvo.variant != rmlvo_new.variant); > + g_assert(rmlvo.options != rmlvo_new.options); > + > + g_assert(strcmp(rmlvo.rules, rmlvo_new.rules) == 0); > + g_assert(strcmp(rmlvo.model, rmlvo_new.model) == 0); > + g_assert(strcmp(rmlvo.layout, rmlvo_new.layout) == 0); > + g_assert(strcmp(rmlvo.variant, rmlvo_new.variant) == 0); > + g_assert(strcmp(rmlvo.options, rmlvo_new.options) == 0); > +} > + > + > +/** > + * Get the default RMLVO set. > + * Set the default RMLVO set. > + * Get the default RMLVO set. > + * Repeat the last two steps. > + * > + * Result: RMLVO set obtained is the same as previously set. > + */ > +static void xkb_set_get_rules_test(void) > +{ > +/* This test failed before XkbGetRulesDftlts changed to strdup. > + We test this twice because the first time using XkbGetRulesDflts we obtain > + the built-in defaults. The unexpected free isn't triggered until the > second > + XkbSetRulesDefaults. > + */ > + XkbRMLVOSet rmlvo = { NULL }; > + XkbRMLVOSet rmlvo_backup; > + > + XkbGetRulesDflts(&rmlvo); > + > + /* pass 1 */ > + XkbSetRulesDflts(&rmlvo); > + XkbGetRulesDflts(&rmlvo); > + > + /* Make a backup copy */ > + rmlvo_backup.rules = strdup(rmlvo.rules); > + rmlvo_backup.layout = strdup(rmlvo.layout); > + rmlvo_backup.model = strdup(rmlvo.model); > + rmlvo_backup.variant = strdup(rmlvo.variant); > + rmlvo_backup.options = strdup(rmlvo.options); > + > + /* pass 2 */ > + XkbSetRulesDflts(&rmlvo); > + > + /* This test is iffy, because strictly we may be comparing against > already > + * freed memory */ > + g_assert(strcmp(rmlvo.rules, rmlvo_backup.rules) == 0); > + g_assert(strcmp(rmlvo.model, rmlvo_backup.model) == 0); > + g_assert(strcmp(rmlvo.layout, rmlvo_backup.layout) == 0); > + g_assert(strcmp(rmlvo.variant, rmlvo_backup.variant) == 0); > + g_assert(strcmp(rmlvo.options, rmlvo_backup.options) == 0); > + > + XkbGetRulesDflts(&rmlvo); > + g_assert(strcmp(rmlvo.rules, rmlvo_backup.rules) == 0); > + g_assert(strcmp(rmlvo.model, rmlvo_backup.model) == 0); > + g_assert(strcmp(rmlvo.layout, rmlvo_backup.layout) == 0); > + g_assert(strcmp(rmlvo.variant, rmlvo_backup.variant) == 0); > + g_assert(strcmp(rmlvo.options, rmlvo_backup.options) == 0); > +} > + > + > +int main(int argc, char** argv) > +{ > + g_test_init(&argc, &argv,NULL); > + g_test_bug_base("https://bugzilla.freedesktop.org/show_bug.cgi?id=";); > + > + g_test_add_func("/xkb/set-get-rules", xkb_set_get_rules_test); > + g_test_add_func("/xkb/get-rules", xkb_get_rules_test); > + g_test_add_func("/xkb/set-rules", xkb_set_rules_test); > + > + return g_test_run(); > +} > -- > 1.6.2.2.447.g4afa7 > > _______________________________________________ > xorg-devel mailing list > [email protected] > http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
