Re: [PATCH v3 5/5] check: add -L <n> parameter to rerun failed tests

[David Disseldorp <ddiss@suse.de>]

On Wed, 6 Jul 2022 23:54:52 +0200, David Disseldorp wrote:

> Thanks for the follow-up feedback, Darrick...
> 
> On Wed, 6 Jul 2022 12:00:07 -0700, Darrick J. Wong wrote:
...
> > >  
> > > +# retain files which would be overwritten in subsequent reruns of the same test
> > > +_stash_fail_loop_files() {
> > > +	local test_seq="$1"
> > > +	local suffix="$2"
> > > +
> > > +	for i in "${REPORT_DIR}/${test_seq}.full" \
> > > +		 "${REPORT_DIR}/${test_seq}.dmesg" \
> > > +		 "${REPORT_DIR}/${test_seq}.out.bad"; do
> > > +		[ -f "$i" ] && cp "$i" "${i}${suffix}"    
> > 
> > I wonder, is there any particular reason to copy the output file and let
> > it get overwritten instead of simply mv'ing it?  
> 
> The copy is left over from an earlier version I had where xunit report
> generation was done after the copy. Looking closer:
> - .full is removed in _begin_fstest()
> - _check_dmesg() overwrites .dmesg and retains on failure or KEEP_DMESG
> - out.bad is removed in the main check loop prior to seq invocation
> - .notrun, .core and .hints are also removed in the check loop at
>   various places before seq (.hints again in _begin_fstest())
> 
> One concern I have in changing this to a move is that external scripts
> may check for presence / parse these files after check invocation. I'd
> considered moving and then copying / symlinking back the .rerun0 files
> on rerun-on-failure loop completion but that's also pretty ugly. IMO
> leaving this as a copy, with the non-suffix file state left to reflect
> the results of the last rerun-on-failure loop, would make the most
> sense for now.

As a follow up here, I plan on squashing in the following change to
cover the extra notrun, core and hints files, and also avoid stale
.rerun# state:

--- a/check
+++ b/check
@@ -560,13 +560,14 @@ _expunge_test()
 
 # retain files which would be overwritten in subsequent reruns of the same test
 _stash_fail_loop_files() {
-       local test_seq="$1"
-       local suffix="$2"
+       local seq_prefix="${REPORT_DIR}/${1}"
+       local cp_suffix="$2"
 
-       for i in "${REPORT_DIR}/${test_seq}.full" \
-                "${REPORT_DIR}/${test_seq}.dmesg" \
-                "${REPORT_DIR}/${test_seq}.out.bad"; do
-               [ -f "$i" ] && cp "$i" "${i}${suffix}"
+       for i in ".full" ".dmesg" ".out.bad" ".notrun" ".core" ".hints"; do
+               rm -f "${seq_prefix}${i}${cp_suffix}"
+               if [ -f "${seq_prefix}${i}" ]; then
+                       cp "${seq_prefix}${i}" "${seq_prefix}${i}${cp_suffix}"
+               fi
        done
 }

Cheers, David