[Spice-devel] [spice-server v4 4/5] test-vdagent: Make test case more useful
Frediano Ziglio
fziglio at redhat.com
Thu Feb 16 12:07:42 UTC 2017
>
> This switches the test to using the GTest API, and add several tests
> related to https://bugzilla.redhat.com/show_bug.cgi?id=1411194
>
> This uses some API not available in glib 2.28, so this checks we have a
> new enough glib before building this test, and disables warnings when
> using too new glib API when building it.
>
> The "multiple-vmc-devices" is based off code written by Frediano Ziglio.
>
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
> configure.ac | 7 ++++
> server/tests/Makefile.am | 20 ++++++++--
> server/tests/test-vdagent.c | 89
> +++++++++++++++++++++++++++++++++++----------
> 3 files changed, 92 insertions(+), 24 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index f04585f..fd9fbae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -148,6 +148,13 @@ AC_SUBST([SPICE_PROTOCOL_MIN_VER])
> GLIB2_REQUIRED=2.28
> GLIB2_ENCODED_VERSION="GLIB_VERSION_2_28"
> PKG_CHECK_MODULES([GLIB2], [glib-2.0 >= $GLIB2_REQUIRED gio-2.0 >=
> $GLIB2_REQUIRED])
> +PKG_CHECK_EXISTS([glib-2.0 >= 2.34], have_glib234="yes", have_glib234="no")
> +dnl disable building of some tests if glib is too old
> +AS_IF([test x"$have_glib234" = "xyes"], [
> + GLIB2_NO_MAXVER_CFLAGS="$GLIB2_CFLAGS"
> + AC_SUBST([GLIB2_NO_MAXVER_CFLAGS])
> +])
> +AM_CONDITIONAL([HAVE_GLIB234], [test x"$have_glib234" = "xyes"])
> GLIB2_CFLAGS="$GLIB2_CFLAGS
> -DGLIB_VERSION_MIN_REQUIRED=$GLIB2_ENCODED_VERSION \
> -DGLIB_VERSION_MAX_ALLOWED=$GLIB2_ENCODED_VERSION"
> AS_VAR_APPEND([SPICE_REQUIRES], [" glib-2.0 >= $GLIB2_REQUIRED gio-2.0 >=
> $GLIB2_REQUIRED"])
> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index af0bd20..6196676 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -1,19 +1,23 @@
> NULL =
>
> -AM_CPPFLAGS = \
> +COMMON_CPPFLAGS = \
> -I$(top_srcdir) \
> -I$(top_srcdir)/server \
> -I$(top_builddir)/server \
> -I$(top_srcdir)/server/tests \
> $(COMMON_CFLAGS) \
> - $(GLIB2_CFLAGS) \
> - $(GOBJECT2_CFLAGS) \
> + $(GOBJECT2_CFLAGS) \
> $(SMARTCARD_CFLAGS) \
> $(SPICE_NONPKGCONFIG_CFLAGS) \
> $(SPICE_PROTOCOL_CFLAGS) \
> $(WARN_CFLAGS) \
> $(NULL)
>
> +AM_CPPFLAGS = \
> + $(COMMON_CPPFLAGS) \
> + $(GLIB2_CFLAGS) \
> + $(NULL)
> +
> if HAVE_AUTOMATED_TESTS
> AM_CPPFLAGS += -DAUTOMATED_TESTS
> endif
I would avoid this change just for a single test.
I used G_GNUC_BEGIN_IGNORE_DEPRECATIONS/G_GNUC_END_IGNORE_DEPRECATIONS
in the code for a similar issue.
Or I'd just remove this limitation for all tests.
Honestly if we can use newer API to do some stuff but we
also provide an implementation for former glib I can't see
the problem but the flag added to check for maximum glib
usage make this harder to do as the deprecated error
is triggered.
> @@ -56,12 +60,20 @@ noinst_PROGRAMS = \
> test-playback \
> test-display-resolution-changes \
> test-two-servers \
> - test-vdagent \
> test-display-width-stride \
> spice-server-replay \
> $(check_PROGRAMS) \
> $(NULL)
>
> +if HAVE_GLIB234
> +check_PROGRAMS += test-vdagent
> +noinst_PROGRAMS += test-vdagent
> +test_vdagent_CPPFLAGS = \
> + $(COMMON_CPPFLAGS) \
> + $(GLIB2_NO_MAXVER_CFLAGS) \
> + $(NULL)
> +endif
> +
You could also add a -U (undefine) like
test_vdagent_CPPFLAGS = $(AM_CPPFLAGS) \
-UGLIB_VERSION_MIN_REQUIRED \
-UGLIB_VERSION_MAX_ALLOWED \
$(NULL)
> if HAVE_GSTREAMER
> noinst_PROGRAMS += test-gst
> endif
> diff --git a/server/tests/test-vdagent.c b/server/tests/test-vdagent.c
> index e06229e..faf5212 100644
> --- a/server/tests/test-vdagent.c
> +++ b/server/tests/test-vdagent.c
> @@ -37,14 +37,6 @@ int ping_ms = 100;
> #define MIN(a, b) ((a) > (b) ? (b) : (a))
> #endif
>
> -static void pinger(SPICE_GNUC_UNUSED void *opaque)
> -{
> - // show_channels is not thread safe - fails if disconnections /
> connections occur
> - //show_channels(server);
> -
> - core->timer_start(ping_timer, ping_ms);
> -}
> -
> static int vmc_write(SPICE_GNUC_UNUSED SpiceCharDeviceInstance *sin,
> SPICE_GNUC_UNUSED const uint8_t *buf,
> int len)
> @@ -62,6 +54,13 @@ static int vmc_read(SPICE_GNUC_UNUSED
> SpiceCharDeviceInstance *sin,
> static unsigned message_size;
> int ret;
>
> + if (pos == sizeof(message)) {
> + g_message("sent whole message");
> + pos++; /* Only print message once */
> + }
> + if (pos > sizeof(message)) {
> + return 0;
> + }
> if (pos == 0) {
> VDIChunkHeader *hdr = (VDIChunkHeader *)message;
> VDAgentMessage *msg = (VDAgentMessage *)&hdr[1];
> @@ -83,9 +82,6 @@ static int vmc_read(SPICE_GNUC_UNUSED
> SpiceCharDeviceInstance *sin,
> ret = MIN(message_size - pos, len);
> memcpy(buf, &message[pos], ret);
> pos += ret;
> - if (pos == message_size) {
> - pos = 0;
> - }
> //printf("vmc_read %d (ret %d)\n", len, ret);
> return ret;
> }
> @@ -105,24 +101,77 @@ static SpiceCharDeviceInterface vmc_interface = {
> .read = vmc_read,
> };
>
> -SpiceCharDeviceInstance vmc_instance = {
> +static SpiceCharDeviceInstance vmc_instance = {
> .subtype = "vdagent",
> };
>
> -int main(void)
> +static void test_multiple_vmc_devices(void)
> {
> - Test *test;
> + SpiceCharDeviceInstance vmc_instances[2] = {
> + { .subtype = "vdagent", },
> + { .subtype = "vdagent", }
> + };
> + int status;
>
> - core = basic_event_loop_init();
> - test = test_new(core);
> + SpiceCoreInterface *core = basic_event_loop_init();
> + Test *test = test_new(core);
>
> + g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
> + "*spice_server_char_device_add_interface: vdagent
> already attached");
> + g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
> + "*spice_server_remove_interface: assertion
> 'char_device->st != NULL'*");
> + vmc_instances[0].base.sif = &vmc_interface.base;
> + spice_server_add_interface(test->server, &vmc_instances[0].base);
> + vmc_instances[1].base.sif = &vmc_interface.base;
> + spice_server_add_interface(test->server, &vmc_instances[1].base);
> + status = spice_server_remove_interface(&vmc_instances[1].base);
> + g_assert_cmpint(status, ==, -1);
> + status = spice_server_remove_interface(&vmc_instances[0].base);
> + g_assert_cmpint(status, ==, 0);
> + g_test_assert_expected_messages();
> + test_destroy(test);
> + basic_event_loop_destroy();
> +}
> +
> +static void test_duplicate_removal(void)
> +{
> + SpiceCoreInterface *core = basic_event_loop_init();
> + Test *test = test_new(core);
> + int status;
> +
> + g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
> + "*spice_server_remove_interface: assertion
> 'char_device->st != NULL'*");
> + vmc_instance.base.sif = &vmc_interface.base;
> + spice_server_add_interface(test->server, &vmc_instance.base);
> + status = spice_server_remove_interface(&vmc_instance.base);
> + g_assert_cmpint(status, ==, 0);
> + status = spice_server_remove_interface(&vmc_instance.base);
> + g_assert_cmpint(status, ==, -1);
> + g_test_assert_expected_messages();
> + test_destroy(test);
> + basic_event_loop_destroy();
> +}
> +
> +static void test_agent_to_server(void)
> +{
> + SpiceCoreInterface *core = basic_event_loop_init();
> + Test *test = test_new(core);
> +
> + g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_MESSAGE, "sent whole
> message");
> vmc_instance.base.sif = &vmc_interface.base;
> spice_server_add_interface(test->server, &vmc_instance.base);
> + g_test_assert_expected_messages();
> + test_destroy(test);
> + basic_event_loop_destroy();
> +}
>
> - ping_timer = core->timer_add(pinger, NULL);
> - core->timer_start(ping_timer, ping_ms);
> +int main(int argc, char *argv[])
> +{
> + g_test_init(&argc, &argv, NULL);
>
> - basic_event_loop_mainloop();
> + g_test_add_func("/server/vdagent/agent-to-server",
> test_agent_to_server);
> + g_test_add_func("/server/vdagent/duplicate-removal",
> test_duplicate_removal);
> + g_test_add_func("/server/vdagent/multiple-vmc-devices",
> test_multiple_vmc_devices);
>
> - return 0;
> + return g_test_run();
> }
The other part looks good
Frediano
More information about the Spice-devel
mailing list