[Spice-devel] [PATCH spice-server v2 3/3] Add a base test for statistic file

[Christophe Fergeau]

On Wed, Nov 16, 2016 at 11:35:07AM +0000, Frediano Ziglio wrote:
> Create a file and add/remove some nodes.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/tests/Makefile.am      |  1 +
>  server/tests/stat-file-test.c | 94 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+)
>  create mode 100644 server/tests/stat-file-test.c
> 
> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index 8580a9a..8351253 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -43,6 +43,7 @@ check_PROGRAMS =				\
>  	test-agent-msg-filter			\
>  	test-loop				\
>  	test-qxl-parsing			\
> +	stat-file-test				\
>  	$(NULL)
>  
>  noinst_PROGRAMS =				\
> diff --git a/server/tests/stat-file-test.c b/server/tests/stat-file-test.c
> new file mode 100644
> index 0000000..364a8fc
> --- /dev/null
> +++ b/server/tests/stat-file-test.c
> @@ -0,0 +1,94 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> +   Copyright (C) 2016 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/>.
> +*/
> +#include <config.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <glib.h>
> +#include <spice.h>
> +
> +#include "stat-file.h"
> +
> +/* GLIB_CHECK_VERSION(2, 40, 0) */
> +#ifndef g_assert_nonnull
> +#define g_assert_nonnull g_assert
> +#endif
> +
> +static void stat_file(void)
> +{
> +    RedStatFile stat_file;
> +    StatNodeRef ref, refs[10];
> +    uint64_t *counter, *counters[10];
> +    int i;
> +    char *filename = NULL;
> +    char name[20];
> +
> +    /* create */
> +    stat_file_init(&stat_file, 10);
> +
> +    g_assert_nonnull(stat_file.shm_name);
> +    filename = strdup(stat_file.shm_name);

I think it's leaked at the end of the test. I'd add a
stat_file_get_shm_name() or such though as this is the only member from
the struct that is needed in order to make the struct opaque.

> +    g_assert(access(stat_file.shm_name, R_OK));
> +
> +    /* fill all nodes */
> +    for (i = 0; i < 10; ++i) {
> +        sprintf(name, "node %d", i);
> +        ref = refs[i] = stat_file_add_node(&stat_file, INVALID_STAT_REF, name, TRUE);

Not a big fan of several statements on the same line.

> +        g_assert_cmpuint(ref,!=,INVALID_STAT_REF);
> +    }
> +
> +    /* should fail */
> +    ref = stat_file_add_node(&stat_file, INVALID_STAT_REF, "invalid", TRUE);
> +    g_assert_cmpuint(ref,==,INVALID_STAT_REF);
> +
> +    /* we should find already present nodes */
> +    for (i = 0; i < 10; ++i) {
> +        sprintf(name, "node %d", (i * 17 + 5) % 10);

Why use this formula? (ditto with the other *23 + 3 below)

Looks good (assuming make check still passes :)


Acked-by: Christophe Fergeau <cfergeau at redhat.com>

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161116/acfa1ddd/attachment.sig>