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

[Frediano Ziglio]

> 
> 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)
> 

Just to delete some nodes here and there.

> Looks good (assuming make check still passes :)
> 

Obviously!
I even tested on RHEL6! (but require a small update)

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

Mumble.. I'll send an update of the entire patchset

Frediano