Re: [ndctl PATCH 1/3] cxl/test: add and use cxl_common_[start|stop] helpers

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Mon, 2023-11-27 at 20:11 -0800, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> CXL unit tests use a mostly common set of commands to setup and tear down
> their test environments. Standardize on a common set and make all unit
> tests that run as part of the CXL suite use the helpers.
> 
> This assures that each test is following the best known practice of
> set up and tear down, and that each is using the existing common
> helper - check_dmesg(). It also allows for expansion of the common
> helpers without the need to touch every unit test.
> 
> Note that this makes all tests have the same execution prerequisites,
> so all tests will skip if a prerequisite for any test is not present.
> At the moment, the extra prereqs are sha256sum and dd, both used by
> cxl-update-firmware.sh. The broad requirement is a good thing, in that
> it enforces correct setup and complete runs of the entire CXL suite.
> 
> cxl-security.sh was excluded from this migration as its setup has more
> in common with the nfit_test and legacy security test than with the
> other CXL unit tests.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  test/common                 | 23 +++++++++++++++++++++++
>  test/cxl-create-region.sh   | 16 ++--------------
>  test/cxl-events.sh          | 18 +++---------------
>  test/cxl-labels.sh          | 16 ++--------------
>  test/cxl-poison.sh          | 17 ++---------------
>  test/cxl-region-sysfs.sh    | 16 ++--------------
>  test/cxl-topology.sh        | 16 ++--------------
>  test/cxl-update-firmware.sh | 17 ++---------------
>  test/cxl-xor-region.sh      | 15 ++-------------
>  9 files changed, 40 insertions(+), 114 deletions(-)
> 
> diff --git a/test/common b/test/common
> index f1023ef20f7e..7a4711593624 100644
> --- a/test/common
> +++ b/test/common
> @@ -150,3 +150,26 @@ check_dmesg()
>         grep -q "Call Trace" <<< $log && err $1
>         true
>  }
> +
> +# cxl_common_start
> +# $1: optional module parameter(s) for cxl-test
> +cxl_common_start()
> +{
> +       rc=77
> +       set -ex
> +       trap 'err $LINENO' ERR
> +       check_prereq "jq"
> +       check_prereq "dd"
> +       check_prereq "sha256sum"
> +       modprobe -r cxl_test
> +       modprobe cxl_test "$1"

This should use "$@". $1 will break if multiple parameters need to be
passed (currently we only ever pass one, so it happens to work, but if
a second param ever gets added this will be surprising).

Rest of this looks good, thanks for doing this cleanup!