[PATCH weston v5 5/6] Converted the config parser test to the new framework.

[Pekka Paalanen]

On Sat, 20 Jun 2015 15:47:47 -0700
"Jon A. Cruz" <jonc at osg.samsung.com> wrote:

> Signed-off-by: Jon A. Cruz <jonc at osg.samsung.com>
> ---
>  Makefile.am                |   9 +-
>  tests/config-parser-test.c | 368 +++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 310 insertions(+), 67 deletions(-)

Hi Jon,

this is a nice example on how your framework is used.


> diff --git a/Makefile.am b/Makefile.am
> index f0ed4d2..8234d2a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1144,7 +1144,14 @@ libtest_runner_la_SOURCES =			\
>  libtest_runner_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
>  
>  config_parser_test_SOURCES = tests/config-parser-test.c
> -config_parser_test_LDADD = libshared.la libtest-runner.la $(COMPOSITOR_LIBS)

Hmm, I'm not sure why COMPOSITOR_LIBS were there in the first place...

> +config_parser_test_LDADD =	\
> +	libshared.la		\
> +	$(COMPOSITOR_LIBS)	\
> +	libzunitc.la		\
> +	libzunitcmain.la
> +config_parser_test_CFLAGS =			\
> +	-Wunused \
> +	-I$(top_srcdir)/tools/zunitc/inc

Needs also $(AM_CFLAGS), which got there by default before.

>  
>  vertex_clip_test_SOURCES =			\
>  	tests/vertex-clip-test.c		\
> diff --git a/tests/config-parser-test.c b/tests/config-parser-test.c
> index f659004..926d291 100644
> --- a/tests/config-parser-test.c
> +++ b/tests/config-parser-test.c
> @@ -27,6 +27,7 @@
>  
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdio.h>
>  #include <string.h>
>  #include <assert.h>
>  #include <errno.h>
> @@ -34,28 +35,70 @@
>  
>  #include "config-parser.h"
>  
> -static struct weston_config *
> -run_test(const char *text)
> -{
> +#include "zunitc/zunitc.h"
> +
> +#define ARRAY_LENGTH(a) ((int) (sizeof (a) / sizeof (a)[0]))

We have helpers.h now.

> +
> +struct fixture_data {
> +	const char *text;
>  	struct weston_config *config;
> +};
> +
> +static struct weston_config *
load_config(const char *text, bool good_content)

Style: function name flush-left.

> +{
> +	struct weston_config *config = NULL;
>  	char file[] = "/tmp/weston-config-parser-test-XXXXXX";
> -	int fd, len;
>  
> -	fd = mkstemp(file);
> -	len = write(fd, text, strlen(text));
> -	assert(len == (int) strlen(text));
> +	ZUC_EXPECT_TRUE(text != NULL);

ZUC_ASSERT_NOT_NULL rather?

> +	if (text) {
> +		int len = 0;
> +		int fd = mkstemp(file);
> +
> +		len = write(fd, text, strlen(text));
> +		ZUC_EXPECT_EQ((int)strlen(text), len);
> +
> +		config = weston_config_parse(file);
> +		close(fd);
> +		unlink(file);
> +
> +		if (good_content)
> +			ZUC_EXPECT_TRUE(config != NULL);
> +		else
> +			ZUC_EXPECT_TRUE(config == NULL);

Rather than a boolean argument and this, why not have the
EXPECT/ASSERT_(NOT_)NULL in the caller?

> +	}
> +	return config;
> +}
>  
> -	config = weston_config_parse(file);
> -	close(fd);
> -	unlink(file);
> +static void *setup_test_config(void *data)
> +{
> +	struct weston_config *config = load_config(data, true);
> +
> +	if (zuc_has_failure())
> +		ZUC_MARK_FATAL("Fixture setup failed.");
>  
>  	return config;
>  }
>  
> -static const char t0[] =
> -	"# nothing in this file...\n";
> +static void *setup_test_config_failing(void *data)
> +{
> +	return load_config(data, false);

What if this actually succeeds in loading the config?
Do we need a clean-up?

> +}
> +
> +static void cleanup_test_config(void *data)
> +{
> +	struct weston_config *config = data;
> +	ZUC_ASSERT_TRUE(config != NULL);
> +	weston_config_destroy(config);
> +}
> +
> +static struct zuc_fixture config_test_t0 = {
> +	.data = "# nothing in this file...\n",
> +	.set_up = setup_test_config,
> +	.tear_down = cleanup_test_config
> +};
>  
> -static const char t1[] =
> +static struct zuc_fixture config_test_t1 = {
> +	.data =
>  	"# comment line here...\n"
>  	"\n"
>  	"[foo]\n"
> @@ -79,130 +122,323 @@ static const char t1[] =
>  	"[bucket]\n"
>  	"material=plastic \n"
>  	"color=red\n"
> -	"contents=sand\n";
> +	"contents=sand\n",
> +	.set_up = setup_test_config,
> +	.tear_down = cleanup_test_config
> +};
>  
>  static const char *section_names[] = {
>  	"foo", "bar", "stuff", "bucket", "bucket"
>  };
>  
> -static const char t2[] =
> +/*
> + * Since these next few won't parse, we don't add the tear_down to
> + * attempt cleanup.
> + */
> +
> +static struct zuc_fixture config_test_t2 = {
> +	.data =
>  	"# invalid section...\n"
> -	"[this bracket isn't closed\n";
> +	"[this bracket isn't closed\n",
> +	.set_up = setup_test_config_failing,
> +};
>  
> -static const char t3[] =
> +static struct zuc_fixture config_test_t3 = {
> +	.data =
>  	"# line without = ...\n"
>  	"[bambam]\n"
> -	"this line isn't any kind of valid\n";
> +	"this line isn't any kind of valid\n",
> +	.set_up = setup_test_config_failing,
> +};
>  
> -static const char t4[] =
> +static struct zuc_fixture config_test_t4 = {
> +	.data =
>  	"# starting with = ...\n"
>  	"[bambam]\n"
> -	"=not valid at all\n";
> +	"=not valid at all\n",
> +	.set_up = setup_test_config_failing,
> +};
>  
> -int main(int argc, char *argv[])
> +ZUC_TEST_F(config_test_t0, comment_only)
>  {
> -	struct weston_config *config;
> -	struct weston_config_section *section;
> -	const char *name;
> -	char *s;
> -	int r, b, i;
> -	int32_t n;
> -	uint32_t u;
> +	struct weston_config *config = data;
> +	ZUC_ASSERT_TRUE(config != NULL);

ZUC_ASSERT_NOT_NULL?

> +}
>  
> -	config = run_test(t0);
> -	assert(config);
> -	weston_config_destroy(config);
> +/** @todo individual t1 tests should have more descriptive names. */

Yeah :-)

>  
> -	config = run_test(t1);
> -	assert(config);
> -	section = weston_config_get_section(config, "mollusc", NULL, NULL);
> -	assert(section == NULL);
> +ZUC_TEST_F(config_test_t1, test001)
> +{
> +	struct weston_config_section *section;
> +	struct weston_config *config = data;
> +	ZUC_ASSERT_TRUE(config != NULL);

ZUC_ASSERT_NON_NULL

> +	section = weston_config_get_section(config,
> +					    "mollusc", NULL, NULL);
> +	ZUC_ASSERT_TRUE(section == NULL);

ZUC_ASSERT_NULL

> +}
> +
> +ZUC_TEST_F(config_test_t1, test002)
> +{
> +	char *s;
> +	int r;
> +	struct weston_config_section *section;
> +	struct weston_config *config = data;
>  
>  	section = weston_config_get_section(config, "foo", NULL, NULL);
>  	r = weston_config_section_get_string(section, "a", &s, NULL);
> -	assert(r == 0 && strcmp(s, "b") == 0);
> +	ZUC_EXPECT_EQ(0, r);
> +	ZUC_EXPECT_EQ(0, strcmp(s, "b"));

ZUC_EXPECT_STREQ?

etc.

Looks good in general.


Thanks,
pq