Re: [PATCH] check: add support for --start-after

From: Luis Chamberlain <mcgrof@kernel.org>
To: Zorro Lang <zlang@redhat.com>
Cc: fstests@vger.kernel.org, aalbersh@redhat.com,
	chandan.babu@oracle.com, amir73il@gmail.com, djwong@kernel.org,
	linux-fsdevel@vger.kernel.org, patches@lists.linux.dev
Subject: Re: [PATCH] check: add support for --start-after
Date: Fri, 16 Feb 2024 08:40:23 -0800	[thread overview]
Message-ID: <Zc-P91LVKAZAgSwy@bombadil.infradead.org> (raw)
In-Reply-To: <20230908052727.ccyxadtibouis74h@zlang-mailbox>

Sorry for some reason I missed this.

On Fri, Sep 08, 2023 at 01:27:27PM +0800, Zorro Lang wrote:
> On Thu, Sep 07, 2023 at 03:10:30PM -0700, Luis Chamberlain wrote:
> > Often times one is running a new test baseline we want to continue to
> > start testing where we left off if the last test was a crash. To do
> > this the first thing that occurred to me was to use the check.time
> > file as an expunge file but that doesn't work so well if you crashed
> > as the file turns out empty.
> > 
> > So instead add super simple argument --start-after which let's you
> > skip all tests until the test infrastructure has "seen" the test
> > you want to skip. This does obviously work best if you are not using
> > a random order, but that is rather implied.
> > 
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > ---
> >  check | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/check b/check
> > index 71b9fbd07522..1ecf07c1cb37 100755
> > --- a/check
> > +++ b/check
> > @@ -18,6 +18,8 @@ showme=false
> >  have_test_arg=false
> >  randomize=false
> >  exact_order=false
> > +start_after=false
> > +start_after_test=""
> >  export here=`pwd`
> >  xfile=""
> >  subdir_xfile=""
> > @@ -80,6 +82,7 @@ check options
> >      -b			brief test summary
> >      -R fmt[,fmt]	generate report in formats specified. Supported formats: xunit, xunit-quiet
> >      --large-fs		optimise scratch device for large filesystems
> > +    --start-after	only start testing after the test specified
> 
> This option conflicts with "-r" option.

I will add a check.

> >      -s section		run only specified section from config file
> >      -S section		exclude the specified section from the config file
> >      -L <n>		loop tests <n> times following a failure, measuring aggregate pass/fail metrics
> > @@ -313,6 +316,11 @@ while [ $# -gt 0 ]; do
> >  				<(sed "s/#.*$//" $xfile)
> >  		fi
> >  		;;
> > +	--start-after)
> > +		start_after=true
> > +		start_after_test="$2"
> 
> Do we really need two variables at here?

We can stick with one but we just need to use:

if [[ "$start_after_test != "" ]]

> 
> > +		shift
> > +		;;
> >  	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
> >  	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
> >  	-l)	diff="diff" ;;
> > @@ -591,6 +599,15 @@ _expunge_test()
> >  {
> >  	local TEST_ID="$1"
> >  
> > +	if $start_after; then
> > +		if [[ "$start_after_test" == ${TEST_ID}* ]]; then
> > +			start_after=false
> > +		fi
> > +		echo "       [skipped]"
> > +		return 0
> > +
> > +	fi
> 
> I can't understand how you use the --start-after. I though you'd like to remove
> all cases before the "start-after" from the running list. But when I saw here,
> I'm a little confused.

I hope the demo in the 2nd version of the patch helps. I'll send a v3.

  Luis