Re: [PATCH spice-server v3 07/12] test-sasl: Add tests for different mechanism names

[Christophe Fergeau <cfergeau@xxxxxxxxxx>]

On Fri, Dec 22, 2017 at 10:07:08AM +0000, Frediano Ziglio wrote:
> @@ -395,16 +427,32 @@ idle_next_test(void *arg)
>  {
>      // end previous test
>      if (test_num >= 0) {
> -        g_assert(encode_called);
> +        const TestData *data = &tests_data[test_num];
> +        if (data->success) {
> +            g_assert(encode_called);
> +        } else {
> +            g_assert(mechlist_called);
> +            g_assert(!encode_called);
> +        }
>          reset_test();
> -        basic_event_loop_quit();
> -        return FALSE;
>      }
>  
>      // start next test
>      ++test_num;
>      alarm(4);
>  
> +    if (test_num >= G_N_ELEMENTS(tests_data)) {
> +        basic_event_loop_quit();
> +        return FALSE;
> +    }
> +
> +    TestData *data = &tests_data[test_num];
> +    if (data->mechlen < 0) {
> +        data->mechlen = strlen(data->mechname);
> +    }
> +    int len = data->mechlen;
> +    printf("\nRunning test %d ('%*.*s' %d)\n", test_num, len, len, data->mechname, len);
> +
>      int sv[2];
>      g_assert_cmpint(socketpair(AF_LOCAL, SOCK_STREAM, 0, sv), ==, 0);

I haven't read over the whole tests yet, but I find the way you iterate
over the tests through idle_next_test() quite hard to read. Adding the
changes below on top of this series make things easier to follow in my
opinion.

diff --git a/server/tests/basic-event-loop.c b/server/tests/basic-event-loop.c
index 6fd8a2934..12205ecef 100644
--- a/server/tests/basic-event-loop.c
+++ b/server/tests/basic-event-loop.c
@@ -130,7 +130,7 @@ SpiceCoreInterface *basic_event_loop_init(void)
 {
     ignore_sigpipe();
     spice_assert(main_context == NULL);
-    main_context = g_main_context_new();
+    main_context = g_main_context_default();
     base_core_interface = event_loop_core;
     base_core_interface.main_context = main_context;
 
diff --git a/server/tests/test-sasl.c b/server/tests/test-sasl.c
index 90f9dc2e6..02317e038 100644
--- a/server/tests/test-sasl.c
+++ b/server/tests/test-sasl.c
@@ -64,9 +64,9 @@ static unsigned test_flags;
 static SpiceCoreInterface *core;
 static SpiceServer *server;
 
-static int test_num = -1;
+static unsigned int test_num;
 
-static gboolean idle_next_test(void *arg);
+static gboolean idle_end_test(void *arg);
 
 static void
 check_sasl_conn(sasl_conn_t *conn)
@@ -298,7 +298,7 @@ start_test(void)
 }
 
 static void
-end_test(void)
+end_tests(void)
 {
     spice_server_destroy(server);
     server = NULL;
@@ -363,15 +363,6 @@ write_u32_err(int fd, uint32_t val)
 #define write_u32(fd, val) \
     g_assert_cmpint(write_u32_err(fd, val), ==, sizeof(uint32_t))
 
-static void
-idle_add(GSourceFunc func, void *arg)
-{
-    GSource *source = g_idle_source_new();
-    g_source_set_callback(source, func, NULL, NULL);
-    g_source_attach(source, basic_event_loop_get_context());
-    g_source_unref(source);
-}
-
 typedef enum {
     STEP_NONE,
     STEP_READ_MECHLIST_LEN,
@@ -525,36 +516,13 @@ client_emulator(void *arg)
     shutdown(sock, SHUT_RDWR);
     close(sock);
 
-    idle_add(idle_next_test, NULL);
+    g_idle_add(idle_end_test, NULL);
 
     return NULL;
 }
 
-// called when a new test has to be run
-static gboolean
-idle_next_test(void *arg)
+static GThread *setup_thread(unsigned int test_num)
 {
-    // end previous test
-    if (test_num >= 0) {
-        const TestData *data = &tests_data[test_num];
-        if (data->success) {
-            g_assert(encode_called);
-        } else {
-            g_assert(mechlist_called);
-            g_assert(!encode_called);
-        }
-        reset_test();
-    }
-
-    // start next test
-    ++test_num;
-    alarm(4);
-
-    if (test_num >= G_N_ELEMENTS(tests_data)) {
-        basic_event_loop_quit();
-        return FALSE;
-    }
-
     TestData *data = &tests_data[test_num];
     if (data->mechlen < 0) {
         data->mechlen = strlen(data->mechname);
@@ -568,11 +536,29 @@ idle_next_test(void *arg)
 
     g_assert(spice_server_add_client(server, sv[0], 0) == 0);
 
-    pthread_t thread;
-    g_assert_cmpint(pthread_create(&thread, NULL, client_emulator, GINT_TO_POINTER(sv[1])), ==, 0);
-    g_assert_cmpint(pthread_detach(thread), ==, 0);
+    GThread *thread = g_thread_new("sasl-thread", client_emulator, GINT_TO_POINTER(sv[1]));
+    return thread;
+}
 
-    return FALSE;
+// called when the next test has to be run
+static gboolean
+idle_end_test(void *arg)
+{
+    basic_event_loop_quit();
+
+    return G_SOURCE_REMOVE;
+}
+
+static void
+check_test_results(unsigned int test_num)
+{
+    const TestData *data = &tests_data[test_num];
+    if (data->success) {
+        g_assert(encode_called);
+    } else {
+        g_assert(mechlist_called);
+        g_assert(!encode_called);
+    }
 }
 
 static void
@@ -582,11 +568,17 @@ sasl_mechs(void)
 
     memset(long_mechname, 'X', sizeof(long_mechname));
 
-    idle_add(idle_next_test, NULL);
-    alarm(4);
-    basic_event_loop_mainloop();
+    for (test_num = 0; test_num < G_N_ELEMENTS(tests_data); test_num++) {
+        GThread *thread;
+        thread = setup_thread(test_num);
+        alarm(4);
+        basic_event_loop_mainloop();
+        g_thread_join(thread);
+        check_test_results(test_num);
+        reset_test();
+    }
 
-    end_test();
+    end_tests();
 }
 
 int

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel