[PATCH weston v6 2/4] Enables output in the JUnit XML format.

[Pekka Paalanen]

On Thu,  2 Jul 2015 23:36:45 -0700
"Jon A. Cruz" <jonc at osg.samsung.com> wrote:

> Adds basic support for optionally outputting in the XML format
> commonly used by JUnit compatible tools.
> 
> This format is supported by default by many tools, including
> the Jenkins build system. It also is more detailed and
> captures more information than the more simplistic TAP
> format.
> 
> Signed-off-by: Jon A. Cruz <jonc at osg.samsung.com>
> ---
>  Makefile.am                           |   9 +
>  configure.ac                          |  26 ++
>  tools/zunitc/src/zuc_junit_reporter.c | 488 ++++++++++++++++++++++++++++++++++
>  tools/zunitc/src/zuc_junit_reporter.h |  38 +++
>  tools/zunitc/src/zunitc_impl.c        |  17 ++
>  5 files changed, 578 insertions(+)
>  create mode 100644 tools/zunitc/src/zuc_junit_reporter.c
>  create mode 100644 tools/zunitc/src/zuc_junit_reporter.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4f0a450..89d9e4c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -982,6 +982,8 @@ libzunitc_la_SOURCES = \
>  	tools/zunitc/src/zuc_context.h		\
>  	tools/zunitc/src/zuc_event.h		\
>  	tools/zunitc/src/zuc_event_listener.h	\
> +	tools/zunitc/src/zuc_junit_reporter.c	\
> +	tools/zunitc/src/zuc_junit_reporter.h	\
>  	tools/zunitc/src/zuc_types.h		\
>  	tools/zunitc/src/zunitc_impl.c		\
>  	shared/helpers.h
> @@ -993,6 +995,13 @@ libzunitc_la_CFLAGS = \
>  libzunitc_la_LIBADD = \
>  	libshared.la
>  
> +if ENABLE_JUNIT_XML
> +libzunitc_la_CFLAGS += \
> +	$(LIBXML2_CFLAGS)
> +libzunitc_la_LIBADD += \
> +	$(LIBXML2_LIBS)
> +endif
> +
>  libzunitcmain_la_SOURCES = \
>  	tools/zunitc/src/main.c
>  
> diff --git a/configure.ac b/configure.ac
> index 404418e..b67df39 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -426,6 +426,31 @@ if test "x$enable_dbus" != "xno"; then
>  fi
>  AM_CONDITIONAL(ENABLE_DBUS, test "x$enable_dbus" = "xyes")
>  
> +# Note that other features might want libxml2, or this feature might use
> +# alternative xml libraries at some point. Therefore the feature and
> +# pre-requisite concepts are split.
> +AC_ARG_ENABLE(junit_xml,
> +	      AS_HELP_STRING([--disable-junit-xml],
> +			     [do not build with JUnit XML output]),,
> +	      enable_junit_xml=auto)
> +if test "x$enable_junit_xml" != "xno"; then
> +	PKG_CHECK_MODULES(LIBXML2,
> +			  [libxml-2.0 >= 2.6],
> +			  have_libxml2=yes,
> +			  have_libxml2=no)
> +	if test "x$have_libxml2" = "xno" -a "x$enable_junit_xml" = "xyes"; then
> +		AC_MSG_ERROR([JUnit XML support explicitly requested, but libxml2 couldn't be found])
> +	fi
> +	if test "x$have_libxml2" = "xyes"; then
> +		enable_junit_xml=yes
> +		AC_DEFINE(HAVE_LIBXML2, [1], [Define if libxml2 is available])

Hi Jon,

HAVE_LIBXML2 is never used, so not needed here.

> +		AC_DEFINE(ENABLE_JUNIT_XML, [1], [Build Weston with JUnit output support])
> +	else
> +		enable_junit_xml=no
> +	fi
> +fi
> +AM_CONDITIONAL(ENABLE_JUNIT_XML, test "x$enable_junit_xml" = "xyes")
> +
>  # ivi-shell support
>  AC_ARG_ENABLE(ivi-shell,
>                AS_HELP_STRING([--disable-ivi-shell],
> @@ -537,6 +562,7 @@ AC_MSG_RESULT([
>  	FBDEV Compositor		${enable_fbdev_compositor}
>  	RDP Compositor			${enable_rdp_compositor}
>  	Screen Sharing			${enable_screen_sharing}
> +	JUnit XML output		${enable_junit_xml}
>  
>  	Raspberry Pi BCM headers	${have_bcm_host}
>  
> diff --git a/tools/zunitc/src/zuc_junit_reporter.c b/tools/zunitc/src/zuc_junit_reporter.c
> new file mode 100644
> index 0000000..7e05fbf
> --- /dev/null
> +++ b/tools/zunitc/src/zuc_junit_reporter.c
> @@ -0,0 +1,488 @@
> +/*
> + * Copyright © 2015 Samsung Electronics Co., Ltd

> + */
> +
> +#include "config.h"
> +
> +#include "zuc_junit_reporter.h"
> +
> +#if ENABLE_JUNIT_XML
> +
> +#include <fcntl.h>
> +#include <libxml/parser.h>
> +#include <memory.h>
> +#include <stdio.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +#include "zuc_event_listener.h"
> +#include "zuc_types.h"
> +
> +#include "shared/zalloc.h"
> +
> +#define XML_FNAME "test_detail.xml"
> +
> +#define ISO_8601_FORMAT "%Y-%m-%dT%H:%M:%SZ"
> +
> +/**
> + * Internal data.
> + */
> +struct junit_data
> +{
> +	int fd;
> +	time_t begin;
> +};
> +
> +/**
> + * Formats a time in milliseconds to the normal JUnit elapsed form, or
> + * NULL if there is a problem.
> + * The caller should release this with free()
> + *
> + * @return the formatted time string upon success, NULL otherwise.
> + */
> +static char *
> +as_duration(long ms);
> +
> +/**
> + * Formats a time in milliseconds to the full ISO-8601 date/time string
> + * format, or NULL if there is a problem.
> + * The caller should release this with free()
> + *
> + * @return the formatted time string upon success, NULL otherwise.
> + */
> +static char *
> +as_iso_8601(time_t const *t);
> +
> +/**
> + * Returns the status string for the tests (run/notrun).
> + *
> + * @param test the test to check status of.
> + * @return the status string.
> + */
> +static char const *
> +get_test_status(struct zuc_test *test);
> +
> +/**
> + * Output the given event.
> + *
> + * @param parent the parent node to add new content to.
> + * @param event the event to write out.
> + */
> +static void
> +emit_event(xmlNodePtr parent, struct zuc_event *event);
> +
> +/**
> + * Output the given test.
> + *
> + * @param parent the parent node to add new content to.
> + * @param test the test to write out.
> + */
> +static void
> +emit_test(xmlNodePtr parent, struct zuc_test *test);
> +
> +/**
> + * Output the given test case.
> + *
> + * @param parent the parent node to add new content to.
> + * @param test_case the test case to write out.
> + */
> +static void
> +emit_case(xmlNodePtr parent, struct zuc_case *test_case);
> +
> +static void
> +destroy(void *data);
> +
> +static void
> +run_started(void *data, int live_case_count, int live_test_count,
> +	    int disabled_count);
> +
> +static void
> +run_ended(void *data, int case_count, struct zuc_case **cases,
> +	  int live_case_count, int live_test_count, int total_passed,
> +	  int total_failed, int total_disabled, long total_elapsed);

Lots of static functions declared instead of defined. Do we really need
these declarations separate from the definition?

> +
> +struct zuc_event_listener *
> +zuc_junit_reporter_create(void)
> +{
> +	struct zuc_event_listener *listener =
> +		zalloc(sizeof(struct zuc_event_listener));
> +
> +	struct junit_data *data = zalloc(sizeof(struct junit_data));

Empty line between decls and code.

> +	data->fd = -1;
> +
> +	listener->data = data;
> +	listener->destroy = destroy;
> +	listener->run_started = run_started;
> +	listener->run_ended = run_ended;
> +
> +	return listener;
> +}
> +
> +void
> +destroy(void *data)
> +{
> +	xmlCleanupParser();
> +
> +	free(data);
> +}

When I read this definition, it makes me think this is not static. A
non-static function with such a very generic name raises an eye-brow.
(An example of how the declarations confuse me here.)

> +
> +void
> +run_started(void *data, int live_case_count, int live_test_count,
> +	    int disabled_count)
> +{
> +	struct junit_data *jdata = data;
> +	jdata->begin = time(NULL);
> +	jdata->fd = open(XML_FNAME, O_WRONLY | O_CLOEXEC | O_CREAT | O_TRUNC,
> +			 S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);

Does it not matter whether open fails? Does something else catch it?

Btw. since this is always creating and truncating the same file, how
does this work when we execute several test programs
one-after-the-other or even in parallel?

> +}
> +
> +char const *
> +get_test_status(struct zuc_test *test)
> +{
> +	if (test->disabled || test->skipped)
> +		return "notrun";
> +	else
> +		return "run";
> +}
> +
> +#define MAX_64BIT_STRLEN 20
> +
> +static void
> +set_attribute(xmlNodePtr node, const char *name, int value)
> +{
> +	xmlChar scratch[MAX_64BIT_STRLEN + 1] = {};
> +	xmlStrPrintf(scratch, sizeof(scratch), BAD_CAST "%d", value);
> +	xmlSetProp(node, BAD_CAST name, scratch);
> +}

> +char *
> +as_duration(long ms)
> +{
> +	char *str = NULL;
> +	if (asprintf(&str, "%1.3f", ms / 1000.0) < 0) {
> +		str = NULL;
> +	} else {
> +		if (!strcmp("0.000", str)) {
> +			free(str);
> +			str = strdup("0");

That's a funny sequence. Why is 0.000 bad?

> +		}
> +	}
> +	return str;
> +}
> +
> +char *
> +as_iso_8601(time_t const *t)
> +{
> +	char *result = NULL;
> +	char buf[32] = {};
> +	struct tm when;
> +	if (gmtime_r(t, &when) != NULL)
> +		if (strftime(buf, sizeof(buf), ISO_8601_FORMAT, &when))
> +			result = strdup(buf);
> +
> +	return result;
> +}
> +
> +#else /* ENABLE_JUNIT_XML */
> +
> +#include "shared/zalloc.h"
> +#include "zuc_event_listener.h"
> +
> +/*
> + * Simple stub version if junit output (including libxml2 support) has
> + * been disabled.
> + */
> +
> +struct zuc_event_listener *
> +zuc_junit_reporter_create(void)
> +{
> +	struct zuc_event_listener *listener = zalloc(sizeof(*listener));
> +
> +	/* Leaving all to default 0/NULL */
> +
> +	return listener;

Should this rather fail than silently not do anything?

> +}
> +
> +#endif /* ENABLE_JUNIT_XML */
> diff --git a/tools/zunitc/src/zuc_junit_reporter.h b/tools/zunitc/src/zuc_junit_reporter.h
> new file mode 100644
> index 0000000..66f3c7b
> --- /dev/null
> +++ b/tools/zunitc/src/zuc_junit_reporter.h
> @@ -0,0 +1,38 @@
> +/*
> + * Copyright © 2015 Samsung Electronics Co., Ltd

> + */
> +
> +#ifndef ZUC_JUNIT_REPORTER_H
> +#define ZUC_JUNIT_REPORTER_H
> +
> +struct zuc_event_listener;
> +
> +/**
> + * Creates an instance of a reporter that will write data in the JUnit
> + * XML format.
> + */
> +struct zuc_event_listener *
> +zuc_junit_reporter_create(void);
> +
> +#endif /* ZUC_JUNIT_REPORTER_H */
> diff --git a/tools/zunitc/src/zunitc_impl.c b/tools/zunitc/src/zunitc_impl.c
> index b8ab0b2..6f591f0 100644
> --- a/tools/zunitc/src/zunitc_impl.c
> +++ b/tools/zunitc/src/zunitc_impl.c


Otherwise looks good to me.


Thanks,
pq