[Spice-devel] [PATCH 1/2] Adding support to automated tests

Alon Levy alevy at redhat.com
Tue Feb 14 07:33:01 PST 2012


On Fri, Feb 03, 2012 at 02:10:03AM -0200, Fabiano Fidêncio wrote:
>     As suggested by Alon, a simple automated test to try to find
>     regressions in Spice code.
>     To use this, compile Spice with --enable-automated-tests and
>     run test_display_streaming passing --automated-tests as parameter.

Thanks, Pushed now with an additional patch to use getopt_long (I wanted
to add some later settings like quiting after a certain number of
drawables for simpler performance tests, and that would be useful for
it). Send the additional patch to the list and to you. Sorry for the
delay,

Alon

> ---
>  Makefile.am                           |    2 +-
>  configure.ac                          |   19 +++++
>  server/tests/Makefile.am              |    4 +
>  server/tests/README                   |    6 ++
>  server/tests/regression_test.py       |   25 +++++++
>  server/tests/test_display_base.c      |  119 ++++++++++++++++++++++++++++++--
>  server/tests/test_display_base.h      |    3 +
>  server/tests/test_display_streaming.c |    8 ++-
>  8 files changed, 176 insertions(+), 10 deletions(-)
>  create mode 100755 server/tests/regression_test.py
> 
> diff --git a/Makefile.am b/Makefile.am
> index f8cfe25..c807359 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -12,4 +12,4 @@ DISTCLEANFILES =                                \
>  
>  EXTRA_DIST = spice.proto spice1.proto spice_codegen.py
>  
> -DISTCHECK_CONFIGURE_FLAGS=--enable-opengl --enable-gui --enable-tunnel --enable-smartcard --with-sasl
> +DISTCHECK_CONFIGURE_FLAGS=--enable-opengl --enable-gui --enable-tunnel --enable-smartcard --with-sasl --enable-automated-tests
> diff --git a/configure.ac b/configure.ac
> index 0f8ad7d..3166eb3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -129,6 +129,13 @@ AC_ARG_ENABLE(client,
>  AS_IF([test x"$enable_client" != "xno"], [enable_client="yes"])
>  AM_CONDITIONAL(SUPPORT_CLIENT, test "x$enable_client" = "xyes")
>  
> +AC_ARG_ENABLE(automated_tests,
> +[  --enable-automated-tests     Enable automated tests using snappy (part of spice--gtk)],,
> +[enable_automated_tests="no"])
> +AS_IF([test x"$enable_automated_tests" != "xno"], [enable_automated_tests="yes"])
> +AM_CONDITIONAL(SUPPORT_AUTOMATED_TESTS, test "x$enable_automated_tests" != "xno")
> +
> +
>  dnl =========================================================================
>  dnl Check deps
>  
> @@ -372,6 +379,16 @@ AM_CONDITIONAL([HAVE_SASL], [test "x$with_sasl2" = "xyes" || test "x$with_sasl"
>  AC_SUBST([SASL_CFLAGS])
>  AC_SUBST([SASL_LIBS])
>  
> +if test "x$enable_automated_tests" = "xyes"; then
> +    AC_MSG_CHECKING([for snappy])
> +    snappy --help >/dev/null 2>&1
> +    if test $? -ne 0 ; then
> +        AC_MSG_RESULT([not found])
> +        AC_MSG_ERROR([snappy was not found, this module is part of spice-gtk andis required to compile this package])
> +    fi
> +    AC_MSG_RESULT([found])
> +fi
> +
>  dnl ===========================================================================
>  dnl check compiler flags
>  
> @@ -550,6 +567,8 @@ echo "
>          Smartcard:                ${enable_smartcard}
>  
>          SASL support:             ${enable_sasl}
> +
> +        Automated tests:          ${enable_automated_tests}
>  "
>  
>  if test $os_win32 == "yes" ; then
> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index 4513322..bc4e00e 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -9,6 +9,10 @@ INCLUDES =                              \
>  	$(SPICE_NONPKGCONFIG_CFLAGS)    \
>  	$(NULL)
>  
> +if SUPPORT_AUTOMATED_TESTS
> +INCLUDES += -DAUTOMATED_TESTS
> +endif
> +
>  AM_LDFLAGS = $(top_builddir)/server/libspice-server.la
>  
>  COMMON_BASE=basic_event_loop.c basic_event_loop.h test_util.h ../../common/backtrace.c
> diff --git a/server/tests/README b/server/tests/README
> index e44251d..2724853 100644
> --- a/server/tests/README
> +++ b/server/tests/README
> @@ -21,3 +21,9 @@ test_fail_on_null_core_interface
>  
>  basic_event_loop.c
>   used by test_just_sockets_no_ssl, can be used by other tests. very crude event loop. Should probably use libevent for better tests, but this is self contained.
> +
> +Automated tests
> +===============
> +
> +test_display_streaming.c
> + this test can be used to check regressions. For this, Spice needs to be compiled with --enable-automated-tests and test_display_streaming needs to be called passing --automated-tests as parameter
> diff --git a/server/tests/regression_test.py b/server/tests/regression_test.py
> new file mode 100755
> index 0000000..e53bf87
> --- /dev/null
> +++ b/server/tests/regression_test.py
> @@ -0,0 +1,25 @@
> +#!/usr/bin/python
> +from subprocess import PIPE, Popen
> +import Image
> +import ImageChops
> +
> +
> +def snappy():
> +    cmd = "snappy -h localhost -p 5912 -o output.ppm"
> +    p = Popen(cmd, shell=True)
> +    p.wait()
> +
> +def verify():
> +    base = Image.open("base_test.ppm")
> +    output = Image.open("output.ppm")
> +    return ImageChops.difference(base, output).getbbox()
> +
> +if __name__ == "__main__":
> +    snappy()
> +    diff = verify()
> +
> +    if diff is None:
> +        print("\033[1;32mSUCCESS: No regressions were found!\033[1;m")
> +    else:
> +        print("\033[1;31mFAIL: Regressions were found!\n\033[1;m"
> +              "\033[1;31m      Please, take a look in your code and go fix it!\033[1;m")
> diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
> index ef5a543..48f59d2 100644
> --- a/server/tests/test_display_base.c
> +++ b/server/tests/test_display_base.c
> @@ -1,9 +1,14 @@
> +
>  #include <config.h>
>  #include <stdlib.h>
>  #include <math.h>
>  #include <string.h>
>  #include <stdio.h>
> +#include <unistd.h>
> +#include <signal.h>
> +#include <wait.h>
>  #include <sys/select.h>
> +#include <sys/types.h>
>  #include <spice/qxl_dev.h>
>  #include "test_display_base.h"
>  #include "red_channel.h"
> @@ -34,15 +39,53 @@ static void test_spice_destroy_update(SimpleSpiceUpdate *update)
>      free(update);
>  }
>  
> -#define WIDTH 320
> +#define WIDTH 640
>  #define HEIGHT 320
>  
> -#define SINGLE_PART 8
> +#define SINGLE_PART 4
>  static const int angle_parts = 64 / SINGLE_PART;
>  static int unique = 1;
>  static int color = -1;
>  static int c_i = 0;
>  
> +/* Used for automated tests */
> +static int control = 3; //used to know when we can take a screenshot
> +static int rects = 16; //number of rects that will be draw
> +static int has_automated_tests = 0; //automated test flag
> +
> +static void sigchld_handler(int signal_num) // wait for the child process and exit
> +{
> +    int status;
> +    wait(&status);
> +    exit(0);
> +}
> +
> +static void regression_test(void)
> +{
> +    pid_t pid;
> +
> +    if (--rects != 0) {
> +        return;
> +    }
> +
> +    rects = 16;
> +
> +    if (--control != 0) {
> +        return;
> +    }
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        char buf[PATH_MAX];
> +        char *envp[] = {buf, NULL};
> +
> +        snprintf(buf, sizeof(buf), "PATH=%s", getenv("PATH"));
> +        execve("regression_test.py", NULL, envp);
> +    } else if (pid > 0) {
> +        return;
> +    }
> +}
> +
>  static void set_cmd(QXLCommandExt *ext, uint32_t type, QXLPHYSICAL data)
>  {
>      ext->cmd.type = type;
> @@ -107,7 +150,13 @@ static SimpleSpiceUpdate *test_spice_create_update_draw(uint32_t surface_id, int
>      if ((t % angle_parts) == 0) {
>          c_i++;
>      }
> -    color = (color + 1) % 2;
> +
> +    if(surface_id != 0) {
> +        color = (color + 1) % 2;
> +    } else {
> +        color = surface_id;
> +    }
> +
>      unique++;
>  
>      update   = calloc(sizeof(*update), 1);
> @@ -116,6 +165,7 @@ static SimpleSpiceUpdate *test_spice_create_update_draw(uint32_t surface_id, int
>  
>      bw       = WIDTH/SINGLE_PART;
>      bh       = 48;
> +
>      bbox.right = bbox.left + bw;
>      bbox.bottom = bbox.top + bh;
>      update->bitmap = malloc(bw * bh * 4);
> @@ -303,6 +353,7 @@ int cursor_notify = NOTIFY_CURSOR_BATCH;
>  #define SURF_WIDTH 320
>  #define SURF_HEIGHT 240
>  uint8_t secondary_surface[SURF_WIDTH * SURF_HEIGHT * 4];
> +int has_secondary;
>  
>  // We shall now have a ring of commands, so that we can update
>  // it from a separate thread - since get_command is called from
> @@ -354,6 +405,10 @@ static void produce_command(void)
>      static int target_surface = 0;
>      static int cmd_index = 0;
>  
> +
> +    if (has_secondary)
> +        target_surface = 1;
> +
>      ASSERT(num_simple_commands);
>  
>      switch (simple_commands[cmd_index]) {
> @@ -361,35 +416,50 @@ static void produce_command(void)
>              path_progress(&path);
>              break;
>          case SIMPLE_UPDATE: {
> -            QXLRect rect = {.left = 0, .right = WIDTH,
> -                            .top = 0, .bottom = HEIGHT};
> -            qxl_worker->update_area(qxl_worker, 0, &rect, NULL, 0, 1);
> +            QXLRect rect = {.left = 0, .right = SURF_WIDTH,
> +                            .top = 0, .bottom = SURF_HEIGHT};
> +            qxl_worker->update_area(qxl_worker, target_surface, &rect, NULL, 0, 1);
>              break;
>          }
> +
>          case SIMPLE_COPY_BITS:
>          case SIMPLE_DRAW: {
>              SimpleSpiceUpdate *update;
> +
> +            if (has_automated_tests)
> +            {
> +                if (control == 0) {
> +                     return;
> +                }
> +
> +                regression_test();
> +            }
> +
>              switch (simple_commands[cmd_index]) {
>                  case SIMPLE_COPY_BITS:
> -                    update = test_spice_create_update_copy_bits(target_surface);
> +                    update = test_spice_create_update_copy_bits(0);
>                      break;
>                  case SIMPLE_DRAW:
> -                    update = test_spice_create_update_draw(target_surface, path.t);
> +                    update = test_spice_create_update_draw(0, path.t);
>                      break;
>              }
>              push_command(&update->ext);
>              break;
>          }
> +
>          case SIMPLE_CREATE_SURFACE: {
>              SimpleSurfaceCmd *update;
>              target_surface = MAX_SURFACE_NUM - 1;
>              update = create_surface(target_surface, SURF_WIDTH, SURF_HEIGHT,
>                                      secondary_surface);
>              push_command(&update->ext);
> +            has_secondary = 1;
>              break;
>          }
> +
>          case SIMPLE_DESTROY_SURFACE: {
>              SimpleSurfaceCmd *update;
> +            has_secondary = 0;
>              update = destroy_surface(target_surface);
>              target_surface = 0;
>              push_command(&update->ext);
> @@ -529,6 +599,8 @@ QXLInterface display_sif = {
>      .set_compression_level = set_compression_level,
>      .set_mm_time = set_mm_time,
>      .get_init_info = get_init_info,
> +
> + /* the callbacks below are called from spice server thread context */
>      .get_command = get_command,
>      .req_cmd_notification = req_cmd_notification,
>      .release_resource = release_resource,
> @@ -572,6 +644,37 @@ SpiceServer* test_init(SpiceCoreInterface *core)
>      path_init(&path, 0, angle_parts);
>      bzero(primary_surface, sizeof(primary_surface));
>      bzero(secondary_surface, sizeof(secondary_surface));
> +    has_secondary = 0;
>      wakeup_timer = core->timer_add(do_wakeup, NULL);
>      return server;
>  }
> +
> +void check_automated(int argc, char **argv)
> +{
> +    struct sigaction sa;
> +
> +    if (argc == 1) {
> +        return;
> +    }
> +
> +    if (argc > 2) {
> +        goto invalid_option;
> +    }
> +
> +    if (strcmp(argv[1], "--automated-tests") != 0) {
> +        goto invalid_option;
> +    }
> +
> +    has_automated_tests = 1;
> +
> +    memset(&sa, 0, sizeof sa);
> +    sa.sa_handler = &sigchld_handler;
> +    sigaction(SIGCHLD, &sa, NULL);
> +
> +    return;
> +
> +invalid_option:
> +    printf("Invalid option!\n"
> +           "Please, check README before run tests!\n" );
> +    exit(0);
> +}
> diff --git a/server/tests/test_display_base.h b/server/tests/test_display_base.h
> index ca94057..4aeaf5f 100644
> --- a/server/tests/test_display_base.h
> +++ b/server/tests/test_display_base.h
> @@ -10,6 +10,9 @@ void test_set_simple_command_list(int* commands, int num_commands);
>  void test_add_display_interface(SpiceServer *server);
>  SpiceServer* test_init(SpiceCoreInterface* core);
>  
> +/* Used for automated tests */
> +void check_automated(int argc, char **argv);
> +
>  // simple queue for commands
>  enum {
>      PATH_PROGRESS,
> diff --git a/server/tests/test_display_streaming.c b/server/tests/test_display_streaming.c
> index 025541d..5052991 100644
> --- a/server/tests/test_display_streaming.c
> +++ b/server/tests/test_display_streaming.c
> @@ -10,13 +10,19 @@
>  int simple_commands[] = {
>      SIMPLE_DRAW,
>      SIMPLE_UPDATE,
> +    PATH_PROGRESS,
> +    SIMPLE_CREATE_SURFACE,
> +    SIMPLE_DESTROY_SURFACE,
>  };
>  
>  SpiceCoreInterface *core;
>  SpiceServer *server;
>  
> -int main(void)
> +int main(int argc, char **argv)
>  {
> +#ifdef AUTOMATED_TESTS
> +    check_automated(argc, argv);
> +#endif
>      core = basic_event_loop_init();
>      server = test_init(core);
>      spice_server_set_streaming_video(server, SPICE_STREAM_VIDEO_ALL);
> -- 
> 1.7.9
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list