diff mbox series

[03/13] fstests: refactor test boilerplate code

Message ID 162317277866.653489.1612159248973350500.stgit@locust (mailing list archive)
State New, archived
Headers show
Series fstests: move test group lists into test files | expand

Commit Message

Darrick J. Wong June 8, 2021, 5:19 p.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

Create two new helper functions to deal with boilerplate test code:

A helper function to set the seq and seqnum variables.  We will expand
on this in the next patch so that fstests can autogenerate group files
from now on.

A helper function to register cleanup code that will run if the test
exits or trips over a standard range of signals.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/preamble |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 new             |   33 ++++++++++++---------------------
 2 files changed, 61 insertions(+), 21 deletions(-)
 create mode 100644 common/preamble

Comments

Chandan Babu R June 10, 2021, 8:44 a.m. UTC | #1
On 08 Jun 2021 at 22:49, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Create two new helper functions to deal with boilerplate test code:
>
> A helper function to set the seq and seqnum variables.  We will expand
> on this in the next patch so that fstests can autogenerate group files
> from now on.
>
> A helper function to register cleanup code that will run if the test
> exits or trips over a standard range of signals.
>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/preamble |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  new             |   33 ++++++++++++---------------------
>  2 files changed, 61 insertions(+), 21 deletions(-)
>  create mode 100644 common/preamble
>
>
> diff --git a/common/preamble b/common/preamble
> new file mode 100644
> index 00000000..63f66957
> --- /dev/null
> +++ b/common/preamble
> @@ -0,0 +1,49 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Oracle.  All Rights Reserved.
> +
> +# Boilerplate fstests functionality
> +
> +# Standard cleanup function.  Individual tests should override this.
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# Install the supplied cleanup code as a signal handler for HUP, INT, QUIT,
> +# TERM, or when the test exits.  Extra signals can be specified as subsequent
> +# parameters.
> +_register_cleanup()
> +{
> +	local cleanup="$1"
> +	shift
> +
> +	test -n "$cleanup" && cleanup="${cleanup}; "
> +	trap "${cleanup}exit \$status" EXIT HUP INT QUIT TERM $*
> +}
> +# Initialize the global seq, seqres, here, tmp, and status variables to their
> +# defaults.  Group memberships are the only arguments to this helper.
> +_begin_fstest()
> +{
> +	if [ -n "$seq" ]; then
> +		echo "_begin_fstest can only be called once!"
> +		exit 1
> +	fi
> +
> +	seq=`basename $0`
> +	seqres=$RESULT_DIR/$seq
> +	echo "QA output created by $seq"
> +
> +	here=`pwd`
> +	tmp=/tmp/$$
> +	status=1	# failure is the default!
> +
> +	_register_cleanup _cleanup
> +
> +	. ./common/rc
> +
> +	# remove previous $seqres.full before test
> +	rm -f $seqres.full
> +
> +}
> diff --git a/new b/new
> index 357983d9..16e7c782 100755
> --- a/new
> +++ b/new
> @@ -153,27 +153,18 @@ cat <<End-of-File >$tdir/$id
>  #
>  # what am I here for?
>  #
> -seq=\`basename \$0\`
> -seqres=\$RESULT_DIR/\$seq
> -echo "QA output created by \$seq"
> -
> -here=\`pwd\`
> -tmp=/tmp/\$\$
> -status=1	# failure is the default!
> -trap "_cleanup; exit \\\$status" 0 1 2 3 15
> -
> -_cleanup()
> -{
> -	cd /
> -	rm -f \$tmp.*
> -}
> -
> -# get standard environment, filters and checks
> -. ./common/rc
> -. ./common/filter
> -
> -# remove previous \$seqres.full before test
> -rm -f \$seqres.full
> +. ./common/preamble
> +_begin_fstest group list here
> +
> +# Override the default cleanup function.
> +# _cleanup()
> +# {
> +# 	cd /
> +# 	rm -f \$tmp.*
> +# }
> +
> +# Import common functions.
> +# . ./common/filter
>  
>  # real QA test starts here
>
Alli June 11, 2021, 9:55 p.m. UTC | #2
On 6/8/21 10:19 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create two new helper functions to deal with boilerplate test code:
> 
> A helper function to set the seq and seqnum variables.  We will expand
> on this in the next patch so that fstests can autogenerate group files
> from now on.
> 
> A helper function to register cleanup code that will run if the test
> exits or trips over a standard range of signals.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Looks straight forward
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/preamble |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>   new             |   33 ++++++++++++---------------------
>   2 files changed, 61 insertions(+), 21 deletions(-)
>   create mode 100644 common/preamble
> 
> 
> diff --git a/common/preamble b/common/preamble
> new file mode 100644
> index 00000000..63f66957
> --- /dev/null
> +++ b/common/preamble
> @@ -0,0 +1,49 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Oracle.  All Rights Reserved.
> +
> +# Boilerplate fstests functionality
> +
> +# Standard cleanup function.  Individual tests should override this.
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# Install the supplied cleanup code as a signal handler for HUP, INT, QUIT,
> +# TERM, or when the test exits.  Extra signals can be specified as subsequent
> +# parameters.
> +_register_cleanup()
> +{
> +	local cleanup="$1"
> +	shift
> +
> +	test -n "$cleanup" && cleanup="${cleanup}; "
> +	trap "${cleanup}exit \$status" EXIT HUP INT QUIT TERM $*
> +}
> +# Initialize the global seq, seqres, here, tmp, and status variables to their
> +# defaults.  Group memberships are the only arguments to this helper.
> +_begin_fstest()
> +{
> +	if [ -n "$seq" ]; then
> +		echo "_begin_fstest can only be called once!"
> +		exit 1
> +	fi
> +
> +	seq=`basename $0`
> +	seqres=$RESULT_DIR/$seq
> +	echo "QA output created by $seq"
> +
> +	here=`pwd`
> +	tmp=/tmp/$$
> +	status=1	# failure is the default!
> +
> +	_register_cleanup _cleanup
> +
> +	. ./common/rc
> +
> +	# remove previous $seqres.full before test
> +	rm -f $seqres.full
> +
> +}
> diff --git a/new b/new
> index 357983d9..16e7c782 100755
> --- a/new
> +++ b/new
> @@ -153,27 +153,18 @@ cat <<End-of-File >$tdir/$id
>   #
>   # what am I here for?
>   #
> -seq=\`basename \$0\`
> -seqres=\$RESULT_DIR/\$seq
> -echo "QA output created by \$seq"
> -
> -here=\`pwd\`
> -tmp=/tmp/\$\$
> -status=1	# failure is the default!
> -trap "_cleanup; exit \\\$status" 0 1 2 3 15
> -
> -_cleanup()
> -{
> -	cd /
> -	rm -f \$tmp.*
> -}
> -
> -# get standard environment, filters and checks
> -. ./common/rc
> -. ./common/filter
> -
> -# remove previous \$seqres.full before test
> -rm -f \$seqres.full
> +. ./common/preamble
> +_begin_fstest group list here
> +
> +# Override the default cleanup function.
> +# _cleanup()
> +# {
> +# 	cd /
> +# 	rm -f \$tmp.*
> +# }
> +
> +# Import common functions.
> +# . ./common/filter
>   
>   # real QA test starts here
>   
>
Eric Biggers June 12, 2021, 12:08 a.m. UTC | #3
On Tue, Jun 08, 2021 at 10:19:38AM -0700, Darrick J. Wong wrote:
> diff --git a/common/preamble b/common/preamble
> new file mode 100644
> index 00000000..63f66957
> --- /dev/null
> +++ b/common/preamble
> @@ -0,0 +1,49 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Oracle.  All Rights Reserved.
> +
> +# Boilerplate fstests functionality
> +
> +# Standard cleanup function.  Individual tests should override this.
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}

This probably should use "rm -rf" so that tests don't need to override this just
because they created directories rather than files.

- Eric
Darrick J. Wong June 12, 2021, 12:34 a.m. UTC | #4
On Fri, Jun 11, 2021 at 05:08:04PM -0700, Eric Biggers wrote:
> On Tue, Jun 08, 2021 at 10:19:38AM -0700, Darrick J. Wong wrote:
> > diff --git a/common/preamble b/common/preamble
> > new file mode 100644
> > index 00000000..63f66957
> > --- /dev/null
> > +++ b/common/preamble
> > @@ -0,0 +1,49 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Oracle.  All Rights Reserved.
> > +
> > +# Boilerplate fstests functionality
> > +
> > +# Standard cleanup function.  Individual tests should override this.
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> 
> This probably should use "rm -rf" so that tests don't need to override this just
> because they created directories rather than files.

Hm.  I've been told (many years) in the past that I shouldn't really be
using recursive rm unless I /know/ that I've created a $tmp.dir
directory.  OTOH that gets rid of a few thousand more lines of cruft...

<shrug> I'll change it to rm -r -f and see what kind of reaction I get.

--D

> 
> - Eric
diff mbox series

Patch

diff --git a/common/preamble b/common/preamble
new file mode 100644
index 00000000..63f66957
--- /dev/null
+++ b/common/preamble
@@ -0,0 +1,49 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Oracle.  All Rights Reserved.
+
+# Boilerplate fstests functionality
+
+# Standard cleanup function.  Individual tests should override this.
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# Install the supplied cleanup code as a signal handler for HUP, INT, QUIT,
+# TERM, or when the test exits.  Extra signals can be specified as subsequent
+# parameters.
+_register_cleanup()
+{
+	local cleanup="$1"
+	shift
+
+	test -n "$cleanup" && cleanup="${cleanup}; "
+	trap "${cleanup}exit \$status" EXIT HUP INT QUIT TERM $*
+}
+# Initialize the global seq, seqres, here, tmp, and status variables to their
+# defaults.  Group memberships are the only arguments to this helper.
+_begin_fstest()
+{
+	if [ -n "$seq" ]; then
+		echo "_begin_fstest can only be called once!"
+		exit 1
+	fi
+
+	seq=`basename $0`
+	seqres=$RESULT_DIR/$seq
+	echo "QA output created by $seq"
+
+	here=`pwd`
+	tmp=/tmp/$$
+	status=1	# failure is the default!
+
+	_register_cleanup _cleanup
+
+	. ./common/rc
+
+	# remove previous $seqres.full before test
+	rm -f $seqres.full
+
+}
diff --git a/new b/new
index 357983d9..16e7c782 100755
--- a/new
+++ b/new
@@ -153,27 +153,18 @@  cat <<End-of-File >$tdir/$id
 #
 # what am I here for?
 #
-seq=\`basename \$0\`
-seqres=\$RESULT_DIR/\$seq
-echo "QA output created by \$seq"
-
-here=\`pwd\`
-tmp=/tmp/\$\$
-status=1	# failure is the default!
-trap "_cleanup; exit \\\$status" 0 1 2 3 15
-
-_cleanup()
-{
-	cd /
-	rm -f \$tmp.*
-}
-
-# get standard environment, filters and checks
-. ./common/rc
-. ./common/filter
-
-# remove previous \$seqres.full before test
-rm -f \$seqres.full
+. ./common/preamble
+_begin_fstest group list here
+
+# Override the default cleanup function.
+# _cleanup()
+# {
+# 	cd /
+# 	rm -f \$tmp.*
+# }
+
+# Import common functions.
+# . ./common/filter
 
 # real QA test starts here