[PATCH blktests v2 0/3] Add --config argument for custom config filenames

[PATCH blktests v2 0/3] Add --config argument for custom config filenames

[André Almeida]

Instead of just using the default config file, one may also find useful to
specify which configuration file would like to use without editing the config
file, like this:

$ ./check --config=tests_nvme
...
$ ./check -c tests_scsi

This pull request solves this. This change means to be optional, in the sense
that the default behavior should not be modified and current setups will not be
affect by this. To check if this is true, I have done the following test:

- Print the value of variables $DEVICE_ONLY, $QUICK_RUN, $TIMEOUT,
  $RUN_ZONED_TESTS, $OUTPUT, $EXCLUDE
  
- Run with the following setups:
    - with a config file in the dir
    - without a config file in the dir
    - configuring using command line arguments

With both original code and with my changes, I validated that the values
remained the same. Then, I used the argument --config=test_config to check that
the values of variables are indeed changing.

This patchset add this feature, update the docs and fix a minor issue with a
command line argument. Also, I have changed "# shellcheck disable=SC1091" to
"# shellcheck source=/dev/null", since it seems the proper way to disable this
check according to shellcheck documentation[1].

Thanks,
André

Changes since v1:
- Reorder commit, so bug fix comes first
- Document multiple -c options behavior


[1] https://github.com/koalaman/shellcheck/wiki/SC1090#exceptions

This patch is also avaible at GitHub:
https://github.com/osandov/blktests/pull/56

André Almeida (3):
  check: Make "device-only" option a valid option
  check: Add configuration file option
  Documentation: Add information about `--config` argument

 Documentation/running-tests.md |  4 +++-
 check                          | 30 +++++++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 6 deletions(-)

-- 
2.23.0

[PATCH blktests v2 1/3] check: Make "device-only" option a valid option

[André Almeida]

"--device-only" option is described at the "Usage" help message and it's
even parsed as an option by the main code. However, since it's not a
parameter of getopt, when trying to use it will trigger a "unrecognized
option". Fix that to allow usage of this option.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 check | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check b/check
index 981058c..a19b9dc 100755
--- a/check
+++ b/check
@@ -650,7 +650,7 @@ Miscellaneous:
 	esac
 }
 
-if ! TEMP=$(getopt -o 'do:q::x:h' --long 'quick::,exclude:,output:,help' -n "$0" -- "$@"); then
+if ! TEMP=$(getopt -o 'do:q::x:h' --long 'device-only,quick::,exclude:,output:,help' -n "$0" -- "$@"); then
 	exit 1
 fi
 
-- 
2.23.0

[PATCH blktests v2 2/3] check: Add configuration file option

[André Almeida]

Add an option to be possible to use a different configuration file
rather than the default "config" file.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 check | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/check b/check
index a19b9dc..97a3c97 100755
--- a/check
+++ b/check
@@ -635,6 +635,10 @@ Test runs:
   -x, --exclude=TEST	 exclude a test (or test group) from the list of
 			 tests to run
 
+  -c, --config=FILE	 use FILE for loading configuration instead of
+			 default 'config' filename. Note that, for multiple
+			 uses of this option, all files will be loaded.
+
 Miscellaneous:
   -h, --help             display this help message and exit"
 
@@ -650,7 +654,7 @@ Miscellaneous:
 	esac
 }
 
-if ! TEMP=$(getopt -o 'do:q::x:h' --long 'device-only,quick::,exclude:,output:,help' -n "$0" -- "$@"); then
+if ! TEMP=$(getopt -o 'do:q::x:c:h' --long 'device-only,quick::,exclude:,output:,config:,help' -n "$0" -- "$@"); then
 	exit 1
 fi
 
@@ -659,10 +663,8 @@ unset TEMP
 
 LOGGER_PROG="$(type -P logger)" || LOGGER_PROG=true
 
-if [[ -r config ]]; then
-	# shellcheck disable=SC1091
-	. config
-fi
+# true if the default configuration file "config" should be used
+DEFAULT_CONFIG=true
 
 # Default configuration.
 : "${DEVICE_ONLY:=0}"
@@ -706,6 +708,17 @@ while true; do
 			EXCLUDE+=("$2")
 			shift 2
 			;;
+		'-c'|'--config')
+			if [[ -r "$2" ]]; then
+				# shellcheck source=/dev/null
+				. "$2"
+				DEFAULT_CONFIG=false
+			else
+				echo "Configuration file $2 not found!"
+				usage err
+			fi
+			shift 2
+			;;
 		'-h'|'--help')
 			usage out
 			;;
@@ -719,6 +732,12 @@ while true; do
 	esac
 done
 
+# if '-c' was not used, try to use the default config file
+if [[ -r config ]] && $DEFAULT_CONFIG; then
+	# shellcheck source=/dev/null
+	. config
+fi
+
 if [[ $QUICK_RUN -ne 0 && ! "${TIMEOUT:-}" ]]; then
 	_error "QUICK_RUN specified without TIMEOUT"
 fi
-- 
2.23.0

[PATCH blktests v2 3/3] Documentation: Add information about `--config` argument

[André Almeida]

Add information about how to use the argument `--config`.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 Documentation/running-tests.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
index 675dac7..2f71e18 100644
--- a/Documentation/running-tests.md
+++ b/Documentation/running-tests.md
@@ -21,7 +21,9 @@ will run all tests in the `loop` group and the `block/002` test.
 
 Test configuration goes in the `config` file at the top-level directory of the
 blktests repository. Test configuration options can also be set as environment
-variables instead of in the `config` file.
+variables instead of in the `config` file. It is also possible to use a
+different file for configuration, using `--config=<file_name>`. Note that, for
+multiple uses of this option, all files will be loaded.
 
 ### Test Devices
 
-- 
2.23.0

Re: [PATCH blktests v2 0/3] Add --config argument for custom config filenames

[Omar Sandoval]

On Wed, Oct 30, 2019 at 07:27:04PM -0300, André Almeida wrote:
> Instead of just using the default config file, one may also find useful to
> specify which configuration file would like to use without editing the config
> file, like this:
> 
> $ ./check --config=tests_nvme
> ...
> $ ./check -c tests_scsi
> 
> This pull request solves this. This change means to be optional, in the sense
> that the default behavior should not be modified and current setups will not be
> affect by this. To check if this is true, I have done the following test:
> 
> - Print the value of variables $DEVICE_ONLY, $QUICK_RUN, $TIMEOUT,
>   $RUN_ZONED_TESTS, $OUTPUT, $EXCLUDE
>   
> - Run with the following setups:
>     - with a config file in the dir
>     - without a config file in the dir
>     - configuring using command line arguments
> 
> With both original code and with my changes, I validated that the values
> remained the same. Then, I used the argument --config=test_config to check that
> the values of variables are indeed changing.
> 
> This patchset add this feature, update the docs and fix a minor issue with a
> command line argument. Also, I have changed "# shellcheck disable=SC1091" to
> "# shellcheck source=/dev/null", since it seems the proper way to disable this
> check according to shellcheck documentation[1].
> 
> Thanks,
> André
> 
> Changes since v1:
> - Reorder commit, so bug fix comes first
> - Document multiple -c options behavior
> 
> 
> [1] https://github.com/koalaman/shellcheck/wiki/SC1090#exceptions
> 
> This patch is also avaible at GitHub:
> https://github.com/osandov/blktests/pull/56
> 
> André Almeida (3):
>   check: Make "device-only" option a valid option
>   check: Add configuration file option
>   Documentation: Add information about `--config` argument
> 
>  Documentation/running-tests.md |  4 +++-
>  check                          | 30 +++++++++++++++++++++++++-----
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> -- 
> 2.23.0
> 

Thanks, applied! I had to rework it a bit so that command line options
would still take precedence over the configuration file, but it should
work now.