Re: [PATCH 2/8] report: derive an xml schema for the xunit report

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Dec 20, 2022 at 10:18:18AM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/12/20 08:01, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > The "xunit" report format emits an XML document that more or less
> > follows the junit xml schema.  However, there are two major exceptions:
> > 
> > 1. fstests does not emit an @errors attribute on the testsuite element
> > because we don't have the concept of unanticipated errors such as
> > "unchecked throwables".
> > 
> > 2. The system-out/system-err elements sound like they belong under the
> > testcase element, though the schema itself imprecisely says "while the
> > test was executed".  The schema puts them under the top-level testsuite
> > element, but we put them under the testcase element.
> > 
> > Define an xml schema for the xunit report format, and update the xml
> > headers to link to the schema file.  This enables consumers of the
> > reports to check mechanically that the incoming document follows the
> > format.
> 
> One thing is, does the official XMLs use tabs as indents?

XML doesn't care one way or another:
https://www.w3.org/TR/xml/#sec-white-space

> We got some lines definitely too long for human to read.
> Any way to make them a little better?

Eh, I guess I could change them to two spaces, given the indentyness of
the schema.

> But overall, it really defines a good standard for us to follow.
> This is definitely a good start.

<nod>

> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> [...]
> > +						<xs:choice minOccurs="0" maxOccurs="2">
> 
> For this, I prefer maxOccurs to be at least 3.
> 
> We have 3 different possible outputs:
> 
> - $seqnum.out.bad
> - $seqnum.full
> - $seqnum.dmesg
> 
> [...]
> 
> > +								</xs:annotation>
> > +								<xs:simpleType>
> > +									<xs:restriction base="pre-string">
> > +										<xs:whiteSpace value="preserve"/>
> > +									</xs:restriction>
> > +								</xs:simpleType>
> > +							</xs:element>
> > +							<xs:element name="system-err" minOccurs="0" maxOccurs="1">
> > +								<xs:annotation>
> > +									<xs:documentation xml:lang="en">Data that was written to standard error while the test was executed</xs:documentation>
> 
> We don't use stderr, but $seqnum.full and $seqnum.dmesg.
> 
> Or can we just rename the "system-out" and "system-err" to something fstests
> specific? E.g.
> 
> - system-output
> - system-full
> - system-dmesg
> 
> Or the system-err/out thing is mostly to keep the compatibility?
> If so, I'd prefer some properties to make it explicit which output
> represents which fstests specific output.

I'll change those, since that's one of the major divergences from the
upstream junit xml schema.  junit says system-out should capture the
stdout of the whole testsuite, not an individual testcase.

> 
> > +								</xs:annotation>
> > +								<xs:simpleType>
> > +									<xs:restriction base="pre-string">
> > +										<xs:whiteSpace value="preserve"/>
> > +									</xs:restriction>
> > +								</xs:simpleType>
> > +							</xs:element>
> > +						</xs:choice>
> > +					</xs:sequence>
> > +					<xs:attribute name="name" type="xs:token" use="required">
> > +						<xs:annotation>
> > +							<xs:documentation xml:lang="en">Name of the test method</xs:documentation>
> 
> Can we update the description to something more fstests specific, better
> with an example?
> Like "test case number, e.g. generic/001".
> 
> This can apply to most description copied from the JUnit doc.

Ok.

> [...]
> > +		<xs:attribute name="timestamp" type="ISO8601_DATETIME_PATTERN" use="required">
> > +			<xs:annotation>
> > +				<xs:documentation xml:lang="en">when the test was executed. Timezone may not be specified.</xs:documentation>
> > +			</xs:annotation>
> 
> This means the start time, thus all our existing timestamp is not following
> the spec already.

I wrote my comments about this part in the thread about patch 1, so
let's leave the discussion there.

--D

> Thanks,
> Qu