Re: [PATCH 4/4] stat: add test for statistic functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2015-12-02 at 11:43 +0000, Frediano Ziglio wrote:
> Make sure code compile with and without statistics enabled (beside
> printing functions).
> 
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  server/tests/.gitignore  |  5 +++
>  server/tests/Makefile.am | 35 ++++++++++++++++++++
>  server/tests/stat-main.c | 49 ++++++++++++++++++++++++++++
>  server/tests/stat-test.c | 83
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 172 insertions(+)
>  create mode 100644 server/tests/stat-main.c
>  create mode 100644 server/tests/stat-test.c
> 
> diff --git a/server/tests/.gitignore b/server/tests/.gitignore
> index 1059247..35b9624 100644
> --- a/server/tests/.gitignore
> +++ b/server/tests/.gitignore
> @@ -9,3 +9,8 @@ spice-server-replay
>  test_display_width_stride
>  test_two_servers
>  test_vdagent
> +stat_test
> +libtest1.a
> +libtest2.a
> +libtest3.a
> +libtest4.a

this should be a little less generically-named. at least libstat_test1 or
something


> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index 96eb39c..f015281 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -44,6 +44,20 @@ noinst_PROGRAMS =				\
>  	test_vdagent				\
>  	test_display_width_stride		\
>  	spice-server-replay			\
> +	stat_test \
> +	$(NULL)
> +
> +TESTS =	\
> +	stat_test				\
> +	$(NULL)
> +
> +check_PROGRAMS = $(TESTS)
> +
> +noinst_LIBRARIES = \
> +	libtest1.a \
> +	libtest2.a \
> +	libtest3.a \
> +	libtest4.a \
>  	$(NULL)
>  
>  test_vdagent_SOURCES =		\
> @@ -113,3 +127,24 @@ spice_server_replay_SOURCES = 			\
>  	basic_event_loop.h			\
>  	test_util.h				\
>  	$(NULL)
> +
> +stat_test_SOURCES = stat-main.c
> +stat_test_LDADD = \
> +	$(LDADD) \
> +	libtest1.a \
> +	libtest2.a \
> +	libtest3.a \
> +	libtest4.a \
> +	$(NULL)
> +
> +libtest1_a_SOURCES = stat-test.c
> +libtest1_a_CPPFLAGS = $(AM_CPPFLAGS) -DCOMP=0 -DWORKER=0 -DTEST_NAME=test1
> +
> +libtest2_a_SOURCES = stat-test.c
> +libtest2_a_CPPFLAGS = $(AM_CPPFLAGS) -DCOMP=0 -DWORKER=1 -DTEST_NAME=test2
> +
> +libtest3_a_SOURCES = stat-test.c
> +libtest3_a_CPPFLAGS = $(AM_CPPFLAGS) -DCOMP=1 -DWORKER=0 -DTEST_NAME=test3
> +
> +libtest4_a_SOURCES = stat-test.c
> +libtest4_a_CPPFLAGS = $(AM_CPPFLAGS) -DCOMP=1 -DWORKER=1 -DTEST_NAME=test4

I assume that you didn't just use the actual names (COMPRESS_STAT) to make sure
the test still works even when the user adds these symbols to their CFLAGS. But
I'd prefer slightly more descriptive names for these defines. Perhaps
TEST_COMPRESS_STAT or similar instead of COMP? Same comment for the generic
TEST_NAME values (perhaps stat_test1 instead of test1, etc)

> diff --git a/server/tests/stat-main.c b/server/tests/stat-main.c
> new file mode 100644
> index 0000000..e7a7df2
> --- /dev/null
> +++ b/server/tests/stat-main.c
> @@ -0,0 +1,49 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> +   Copyright (C) 2015 Red Hat, Inc.
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> +*/
> +
> +/* check statistics
> + */
> +
> +#include <config.h>
> +#include <glib.h>
> +
> +typedef void test_func_t(void);
> +
> +test_func_t test1;
> +test_func_t test2;
> +test_func_t test3;
> +test_func_t test4;
> +
> +static test_func_t *test_funcs[] = {
> +    test1,
> +    test2,
> +    test3,
> +    test4,
> +    NULL
> +};
> +
> +int main(void)
> +{
> +    test_func_t **func_p;
> +
> +    for (func_p = test_funcs; *func_p; ++func_p) {
> +        (*func_p)();
> +    }
> +    return 0;
> +}
> +
> diff --git a/server/tests/stat-test.c b/server/tests/stat-test.c
> new file mode 100644
> index 0000000..db267e7
> --- /dev/null
> +++ b/server/tests/stat-test.c
> @@ -0,0 +1,83 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> +   Copyright (C) 2015 Red Hat, Inc.
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> +*/
> +
> +/* check statistics
> + */
> +
> +#include <config.h>
> +
> +#undef COMPRESS_STAT
> +#undef RED_WORKER_STAT
> +
> +#if COMP
> +#define COMPRESS_STAT
> +#endif
> +
> +#if WORKER
> +#define RED_WORKER_STAT
> +#endif
> +
> +#include <unistd.h>
> +#include <glib.h>
> +#include "../stat.h"
> +
> +#ifndef TEST_NAME
> +#error TEST_NAME must be defined!
> +#endif
> +
> +void TEST_NAME(void)
> +{
> +    stat_info_t info;
> +    stat_start_time_t start_time;
> +
> +#if !defined(COMPRESS_STAT) && !defined(RED_WORKER_STAT)
> +    G_STATIC_ASSERT(sizeof(start_time) == 0);
> +    G_STATIC_ASSERT(sizeof(info) == 0);
> +#endif
> +
> +    stat_init(&info, "test", CLOCK_MONOTONIC);
> +    stat_start_time_init(&start_time, &info);
> +    usleep(2);
> +    stat_add(&info, start_time);
> +
> +#ifdef RED_WORKER_STAT
> +    g_assert_cmpuint(info.count, ==, 1);
> +    g_assert_cmpuint(info.min, ==, info.max);
> +    g_assert_cmpuint(info.min, >=, 2000);
> +    g_assert_cmpuint(info.min, <, 100000000);
> +#endif
> +
> +    stat_reset(&info);
> +
> +    stat_compress_init(&info, "test", CLOCK_MONOTONIC);
> +    stat_start_time_init(&start_time, &info);
> +    usleep(2);
> +    stat_compress_add(&info, start_time, 100, 50);
> +    usleep(1);
> +    stat_compress_add(&info, start_time, 1000, 500);
> +
> +#ifdef COMPRESS_STAT
> +    g_assert_cmpuint(info.count, ==, 2);
> +    g_assert_cmpuint(info.min, !=, info.max);
> +    g_assert_cmpuint(info.min, >=, 2000);

It seems strange to me that stat_compress_init() above does not reset the start
time when it sets a new clock. I would have expected this value to be 1000, not
2000...

> +    g_assert_cmpuint(info.min, <, 100000000);
> +    g_assert_cmpuint(info.total, >=, 5000);

...and I would have expected this value to be 3000, not 5000.

> +    g_assert_cmpuint(info.orig_size, ==, 1100);
> +    g_assert_cmpuint(info.comp_size, ==, 550);
> +#endif
> +}
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]