[PATCH v4 4/6] Enables output in the JUnit XML format.

[Peter Hutterer]

On Thu, Jun 11, 2015 at 10:01:30PM -0700, Jon A. Cruz wrote:
> Adds basic support fo 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>
> ---
[...]
> +	case ZUC_OP_TERMINATE:
> +	{
> +		char const *level = (event->val1 == 0) ? "error"
> +			: (event->val1 == 1) ? "warning"
> +			: "note";

switch() statement?
also, 0 and 1? we don't have enums for the levels?

> +		if (asprintf(&msg, "%s:%d: %s: %s\n",
> +			     event->file, event->line, level,
> +			     event->expr1) < 0) {
> +			msg = NULL;
> +		}
> +		break;
> +	}
> +	case ZUC_OP_TRACEPOINT:
> +		if (asprintf(&msg, "%s:%d: note: %s\n",
> +			     event->file, event->line, event->expr1) < 0) {
> +			msg = NULL;
> +		}
> +		break;
> +	default:
> +		if (asprintf(&msg, "%s:%d: error: "
> +			     "Expected: (%s) %s (%s), actual: %ld vs %ld\n",
> +			     event->file, event->line,
> +			     event->expr1, zuc_get_opstr(event->op),
> +			     event->expr2, event->val1, event->val2) < 0) {
> +			msg = NULL;
> +		}
> +	}
> +
> +	if ((event->op == ZUC_OP_TERMINATE) && (event->val1 > 1)) {

> +		dprintf(fd, "      <skipped/>\n");
> +	} else {
> +		dprintf(fd, "      <failure");
> +		if (msg) {
> +			dprintf(fd, " message=\"");
> +			emit_escaped(fd, msg);
> +			dprintf(fd, "\"");

why not a dprintf("<failure message="%s">\n<!CDATA...") and compress it into
a single printf statement.
but regardless of that, I fully agree with daniels, not using a lib to write
out xml is just going to be a PITA long-term.

> +		}
> +		dprintf(fd, " type=\"\"");
> +		if (msg) {
> +			dprintf(fd, " >");
> +			dprintf(fd, "<![CDATA[%s]]>", msg);
> +			dprintf(fd, "</failure>\n");
> +		} else {
> +			dprintf(fd, " />");
> +		}

this is generated xml, in a format not intended for human eyes*, always read
by a parser. why do we even care that it's <failure /> vs
<failure></failure> in the empty case? it just complicates the code.

anyway, please use an xml writer library for this.


Cheers,
   Peter

* haha, xml for human eyes. hahahaha.
  /me wipes away tears of laughter