[Spice-devel] [spice-server v2] tests: Automatically determine free port to use

[Frediano Ziglio]

> 
> Currently, the port used by most tests is hardcoded to 5912. However,
> the test suite can be run in parallel, so if 2 tests run in parallel,
> the 2nd one is not going to be able to bind to port 5912 and will fail.
> 
> After this commit, test_new() will try to find a free port between 5912
> and 5922 and will abort if it can't find any.
> 
> The issue can be reproduced by adding a usleep(1000000) to the beginning
> of test_destroy().
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
> Changes since v1:
> - don't leak memory calling spice_init() multiple times on the same
>   instance
> - move BASE_PORT to a better place
> - use for loop rather than while()
> - remove  'ignore_bind_failure' fatal log handler when no longer needed
> 
> 
>  server/tests/test-display-base.c | 54
>  ++++++++++++++++++++++++++++++++--------
>  server/tests/test-display-base.h |  1 -
>  server/tests/test-two-servers.c  |  2 +-
>  3 files changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/server/tests/test-display-base.c
> b/server/tests/test-display-base.c
> index 289aa9840..ab6fc3b36 100644
> --- a/server/tests/test-display-base.c
> +++ b/server/tests/test-display-base.c
> @@ -32,6 +32,7 @@
>  #include <spice/qxl_dev.h>
>  
>  #include "test-display-base.h"
> +#include "test-glib-compat.h"
>  #include "red-channel.h"
>  
>  #ifndef PATH_MAX
> @@ -899,24 +900,60 @@ void test_set_command_list(Test *test, Command
> *commands, int num_commands)
>      test->num_commands = num_commands;
>  }
>  
> +static gboolean ignore_bind_failures(const gchar *log_domain,
> +                                     GLogLevelFlags log_level,
> +                                     const gchar *message,
> +                                     gpointer user_data)
> +{
> +    if (!g_str_equal (log_domain, G_LOG_DOMAIN)) {
> +        return true;
> +    }
> +    if ((log_level & G_LOG_LEVEL_WARNING) == 0)  {
> +        return true;
> +    }
> +    if (strstr(message, "reds_init_socket: binding socket to ") == NULL) {
> +        g_print("XXX [%s]\n", message);
> +        return true;
> +    }
>  
> -Test* test_new_with_port(SpiceCoreInterface* core, int port)
> +    return false;
> +}
> +
> +#define BASE_PORT 5912
> +
> +Test* test_new(SpiceCoreInterface* core)
>  {
>      Test *test = spice_new0(Test, 1);
> -    SpiceServer* server = spice_server_new();
> +    int port = -1;
>  
>      test->qxl_instance.base.sif = &display_sif.base;
>      test->qxl_instance.id = 0;
>  
>      test->core = core;
> -    test->server = server;
>      test->wakeup_ms = 1;
>      test->cursor_notify = NOTIFY_CURSOR_BATCH;
>      // some common initialization for all display tests
> +    port = BASE_PORT;
> +
> +    g_test_log_set_fatal_handler(ignore_bind_failures, NULL);
> +    for (port = BASE_PORT; port < BASE_PORT + 10; port++) {
> +        SpiceServer* server = spice_server_new();
> +        spice_server_set_noauth(server);
> +        spice_server_set_port(server, port);
> +        if (spice_server_init(server, core) == 0) {
> +            test->server = server;
> +            break;
> +        }
> +        spice_server_destroy(server);
> +    }
> +
> +    if (port >= BASE_PORT + 10) {
> +        g_assert_not_reached();
> +        return NULL;
> +    }
> +

maybe a

   g_assert(test->server);

so you can easily increase the range changing just the for line?

>      printf("TESTER: listening on port %d (unsecure)\n", port);
> -    spice_server_set_port(server, port);
> -    spice_server_set_noauth(server);
> -    spice_server_init(server, core);
> +    g_test_log_set_fatal_handler(NULL, NULL);
>  
>      cursor_init();
>      path_init(&path, 0, angle_parts);
> @@ -925,11 +962,6 @@ Test* test_new_with_port(SpiceCoreInterface* core, int
> port)
>      return test;
>  }
>  
> -Test *test_new(SpiceCoreInterface *core)
> -{
> -    return test_new_with_port(core, 5912);
> -}
> -
>  void test_destroy(Test *test)
>  {
>      spice_server_destroy(test->server);
> diff --git a/server/tests/test-display-base.h
> b/server/tests/test-display-base.h
> index a80f03e78..1a4f20c5b 100644
> --- a/server/tests/test-display-base.h
> +++ b/server/tests/test-display-base.h
> @@ -134,7 +134,6 @@ void test_set_simple_command_list(Test *test, const int
> *command, int num_comman
>  void test_set_command_list(Test *test, Command *command, int num_commands);
>  void test_add_display_interface(Test *test);
>  void test_add_agent_interface(SpiceServer *server); // TODO - Test *test
> -Test* test_new_with_port(SpiceCoreInterface* core, int port);
>  Test* test_new(SpiceCoreInterface* core);
>  void test_destroy(Test *test);
>  
> diff --git a/server/tests/test-two-servers.c
> b/server/tests/test-two-servers.c
> index 92935528e..40a0e5717 100644
> --- a/server/tests/test-two-servers.c
> +++ b/server/tests/test-two-servers.c
> @@ -42,7 +42,7 @@ int main(void)
>  
>      core = basic_event_loop_init();
>      t1 = test_new(core);
> -    t2 = test_new_with_port(core, 5913);
> +    t2 = test_new(core);
>      //spice_server_set_image_compression(server,
>      SPICE_IMAGE_COMPRESSION_OFF);
>      test_add_display_interface(t1);
>      test_add_display_interface(t2);

either way,

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano