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

Darrick J. Wong
2023-02-14T18:54:11Z

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