Re: [RFC PATCH 0/2] add option to rerun failed tests

[David Disseldorp <ddiss@suse.de>]

On Fri, 24 Jun 2022 11:18:58 -0400, Theodore Ts'o wrote:

> On Fri, Jun 24, 2022 at 10:32:43AM +0200, David Disseldorp wrote:
> > Yes, I forgot to mention that, sorry. As Zorro indicated, these were
> > done atop the v2022.06.12 tag with the following series applied:
> > https://lore.kernel.org/fstests/20220620192934.21694-1-ddiss@suse.de/  
> 
> Got it, thanks.  Sorry, I had forgotten that we had the next branch now.
> 
> I'll try to do a full review once I'm able to give the patches a spin.
> 
> > > <testcase classname="xfstests.global" name="generic/476" time="354">
> > > 		<failure message="Test  failed, reason unknown" type="TestFail" />
> > > 		<system-out>
> > > 		...
> > > 	</testcase>
> > > <testcase classname="xfstests.global" name="generic/476" time="343">
> > > 	</testcase>
> > > <testcase classname="xfstests.global" name="generic/476" time="353">
> > > 	</testcase>
> > > 	...  
> > 
> > That seems sensible, I'll add this functionality.  
> 
> I *think* that should happen automatically when _make_testcase_report
> gets called after each iteration.  So that might be easier than having
> to do any kind of special case handling.  (Which is why that was going
> to be how I was planning on tackling it before you went ahead and
> implemented --- thanks for that!!)

It does, but I've messed around with a few things in that code path, so
just need to make sure that this works as expected :). It should be
working this way in the v2 patchset that I'm about to send...

> > > As far as haivng the .bad and .full files, I agree that some kind of
> > > .rerun-NN suffix would make a lot of sense.  
> > 
> > I had a bit of a play with this and it does get a bit ugly if we want to
> > prefix things like .dmesg as well. The xunit rerun entries will already
> > capture everything, but I suppose it's still needed for those not using
> > xunit reports.  
> 
> Well, actually, one of the things on my TODO list was to implement a
> new report type which would removed the xunit <system-out> fields from
> the xunit file.  The reason behind that is sometimes the the
> NNN.out.bad files can get huge --- and the Python library for parsing
> junit XML files has a safety mechanism which will error out if a field
> is larger than 10MB, to prevent some denial of service attacks.  And
> I've had some XFS NNN.out.bad files get to be 30MB or larger!

Ouch, that does sound hard to parse. One thing I also noticed is that a
stray "]]>" CDATA terminator in the any of the captured content will
likely also cause some parsing headaches, so should be filtered.

> When that happens, it causes the Python script I use to parse the XML
> file to fail.  In addition, since I already have a different mechanism
> for saving the full set of test artifiacts ---- sometimes having the
> NNN.full file is really useful for root causing the failure --- having
> two copies of the out.bad files in both the Xunit file and in my test
> artifacts tarball is a bit of a waste.
> 
> I had a POC which implemented this, but then Darrick had a feature
> request, since for his workflow, it would be useful if saved only the
> first N lines and last N lines in the xunit file, since that's
> typically sufficient to figure out what's going on.  And I haven't had
> a chance to get back to it.

Given that the extra debugging details are already there in the current
xunit output, I think we may be stuck with them. That said, it should be
pretty straightforward to add a new "xunit-brief" or similar report type
under the (currently single-purpose) report API.

Cheers, David