[icedtea-web] xml output for junit, transformation sheets for daily report

Fri May 6 17:21:24 PDT 2011

On 05/04/2011 07:29 AM, Jiri Vanek wrote:
> On 04/30/2011 03:26 PM, Jiri Vanek wrote:
>> On 04/29/2011 10:56 PM, Omair Majid wrote:
>>> On 04/29/2011 05:22 AM, Jiri Vanek wrote:
>>>> This patch add functionality to export xml report form junit testsuite,
>>>> provides some basic nice transformation for make xml fast readable and
>>>> navigation-able. Also add filtering of inner classes and jnlp from
>>>> tests-source package
>>>>
>>>> Pavel, is xml-content enough for you?
>>>>
>>>
>>> Looks like you forgot to attach the
>>> tests/junit-runner/XmlOutputListener.java file. The changes to
>>> CommandLine.java look fine to me, but I would like to see the rest of
>>> the patch ;)
>>>
>>> Is the xml schema based on an existing schema or is it completely new?
>>>
>>
>> No. it is minimalistic to our purposes. Should follow junit ant-xml
>> output? Or some else?
>>
>
> Refresh call for review:)
>

Let me being by saying that I don't know enough about 
Javascript/XML/XSLT to offer good criticism. I will try my best though.

> As Omair suggested, i have added class which exports xml-output
> following junit-xml output. As I have not found official dtd or schmea,
> I was careful which elements/attributes to output as resources on the
> web were not united. Current resul is quit sufficient - the only thing I
> miss are by-class statistics and date. I would like to break  "junit
> dtd" (as there is none) and add those elements from my previous xml
> output. This step will not break possible usage of tools working with
> ant-xml output.
>

It's really up to you. I thought junit-like xml might be better since we 
could reuse tools for junit. But since you are already writing html 
reports and everything, I dont think it matters too much. Breaking 
junit's dtd (even if you know exactly what it is) is fine too.

> In this patch are both xml exporters so reviewer can judge weather to
> keep both, merge them all keep just one (probably junit-like).
>
> I think Pavel do not care which one will appears at the and:)
>

I will leave it to others (especially you and ptisnovs) to judge that. I 
am fine with keeping both (if you want to maintain it, that is), though 
I would like the default output to be just one format (with a switch for 
the other format).

> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/index.html	Wed May 04 13:16:27 2011 +0200
> @@ -0,0 +1,35 @@
> +<html>
> +<head>
> +<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
> +<script src="styles/index.js">
> +</script>
> +<link href="styles/report.css" rel="stylesheet" type="text/css"/>
> +</head>
> +<body onload="xslt('styles/report.xsl','tests.build/netx/unit/tests-output.xml','wholePage1');
> +		xslt('styles/jreport.xsl','tests.build/netx/unit/junit-like-tests-output.xml','wholePage2');
> +		xslt('styles/report.xsl','tests.build/netx/dist/tests-output.xml','wholePage3');
> +		xslt('styles/jreport.xsl','tests.build/netx/dist/junit-like-tests-output.xml','wholePage4');">
> +<div>

Is there a way to avoid hardcoding these paths? This html file is placed 
in the source directory, I think it breaks with out-of-tree builds. 
Perhaps this file should be stored in $(abs_top_srcdir)/tests/ and after 
the tests are run, an updated version is placed in $(abs_top_builddir)/ 
($(abs_top_builddir)/tests.build)).

> diff -r a9729e1bb762 styles/index.js
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/styles/index.js	Wed May 04 13:16:27 2011 +0200
> @@ -0,0 +1,67 @@
> +
> +if(typeof String.prototype.trim !== 'function') {   String.prototype.trim = function() {     return this.replace(/^\s+|\s+$/g, '');    } }
> +
> +
> +function negateId(which){
> +	var e = document.getElementById(which);
> + 		if (e.style.display=="block") {
> +			e.style.display="none"
> + 		}else{
> +			 e.style.display="block"
> +		 }
> +	 }
> +
> +
> + function setClass(which,what) {

Perhaps setDisplay might be a better name?

> +	 var e = document.getElementsByClassName(which);
> +		 for ( var i = 0; i<  e.length; i++ ){
> +			 e[i].style.display=what
> +			 }
> +		 }
> +
> +
> +
> +
> + function xslt(sheet,data,dest) {
> +	 var sheetName=sheet;
> +	 var xmlName=data;
> +	 var htmlDest=dest;
> +	 // code for IE
> +	 if (window.ActiveXObject) {
> +		 var XML = new ActiveXObject("MSXML2.FreeThreadedDomDocument");
> +		 XML.async = "false";
> +		 XML.load(xmlName);
> +		 var XSL = new ActiveXObject("MSXML2.FreeThreadedDomDocument");
> +		 XSL.async = "false";
> +		 XSL.load(sheetName);
> +		 var XSLTCompiled = new ActiveXObject("MSXML2.XSLTemplate");
> +		 //Add the stylesheet information
> +		 XSLTCompiled.stylesheet = XSL.documentElement;
> +		 //Create the XSLT processor
> +		 var msShit = XSLTCompiled.createProcessor();
> +		 msShit.input = XML

This is the funniest thing I saw today :) Still, I dont think this is 
appropriate. Please rename this variable. Personally, I am just fine 
with leaving out the entire "if IE" block.

> diff -r a9729e1bb762 tests/junit-runner/JunitLikeXmlOutputListener.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/junit-runner/JunitLikeXmlOutputListener.java	Wed May 04 13:16:27 2011 +0200

> diff -r a9729e1bb762 tests/junit-runner/XmlOutputListener.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/junit-runner/XmlOutputListener.java	Wed May 04 13:16:27 2011 +0200

Two comments on the files above: please add a little more documentation, 
and please use System.nanoTime() which is more resistant to change in 
clock times.

Everything else looks good enough to me (though I am quite unfamiliar 
with most of the things here).

Cheers,
Omair