* [PATCH blktests 0/3] Add --config argument for custom config filenames
@ 2019-10-29 20:09 André Almeida
2019-10-29 20:09 ` [PATCH blktests 1/3] check: Add configuration file option André Almeida
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: André Almeida @ 2019-10-29 20:09 UTC (permalink / raw)
To: linux-block; +Cc: osandov, kernel, krisman, 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é
[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: Add configuration file option
Documentation: Add information about `--config` argument
check: Make "device-only" option a valid option
Documentation/running-tests.md | 3 ++-
check | 29 ++++++++++++++++++++++++-----
2 files changed, 26 insertions(+), 6 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH blktests 1/3] check: Add configuration file option
2019-10-29 20:09 [PATCH blktests 0/3] Add --config argument for custom config filenames André Almeida
@ 2019-10-29 20:09 ` André Almeida
2019-10-30 21:16 ` Omar Sandoval
2019-10-29 20:09 ` [PATCH blktests 2/3] Documentation: Add information about `--config` argument André Almeida
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: André Almeida @ 2019-10-29 20:09 UTC (permalink / raw)
To: linux-block; +Cc: osandov, kernel, krisman, 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 981058c..936b0c3 100755
--- a/check
+++ b/check
@@ -635,6 +635,9 @@ 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.
+
Miscellaneous:
-h, --help display this help message and exit"
@@ -650,7 +653,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:c:h' --long 'quick::,exclude:,output:,config:,help' -n "$0" -- "$@"); then
exit 1
fi
@@ -659,10 +662,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 +707,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 +731,13 @@ 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH blktests 2/3] Documentation: Add information about `--config` argument
2019-10-29 20:09 [PATCH blktests 0/3] Add --config argument for custom config filenames André Almeida
2019-10-29 20:09 ` [PATCH blktests 1/3] check: Add configuration file option André Almeida
@ 2019-10-29 20:09 ` André Almeida
2019-10-29 20:09 ` [PATCH blktests 3/3] check: Make "device-only" option a valid option André Almeida
2019-10-30 21:18 ` [PATCH blktests 0/3] Add --config argument for custom config filenames Omar Sandoval
3 siblings, 0 replies; 8+ messages in thread
From: André Almeida @ 2019-10-29 20:09 UTC (permalink / raw)
To: linux-block; +Cc: osandov, kernel, krisman, André Almeida
Add information about how to use the argument `--config`.
Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
Documentation/running-tests.md | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
index 675dac7..d5921be 100644
--- a/Documentation/running-tests.md
+++ b/Documentation/running-tests.md
@@ -21,7 +21,8 @@ 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>`.
### Test Devices
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH blktests 3/3] check: Make "device-only" option a valid option
2019-10-29 20:09 [PATCH blktests 0/3] Add --config argument for custom config filenames André Almeida
2019-10-29 20:09 ` [PATCH blktests 1/3] check: Add configuration file option André Almeida
2019-10-29 20:09 ` [PATCH blktests 2/3] Documentation: Add information about `--config` argument André Almeida
@ 2019-10-29 20:09 ` André Almeida
2019-10-30 21:18 ` [PATCH blktests 0/3] Add --config argument for custom config filenames Omar Sandoval
3 siblings, 0 replies; 8+ messages in thread
From: André Almeida @ 2019-10-29 20:09 UTC (permalink / raw)
To: linux-block; +Cc: osandov, kernel, krisman, 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 936b0c3..6b4c0d0 100755
--- a/check
+++ b/check
@@ -653,7 +653,7 @@ Miscellaneous:
esac
}
-if ! TEMP=$(getopt -o 'do:q::x:c:h' --long 'quick::,exclude:,output:,config:,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
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH blktests 1/3] check: Add configuration file option
2019-10-29 20:09 ` [PATCH blktests 1/3] check: Add configuration file option André Almeida
@ 2019-10-30 21:16 ` Omar Sandoval
2019-10-30 22:05 ` André Almeida
0 siblings, 1 reply; 8+ messages in thread
From: Omar Sandoval @ 2019-10-30 21:16 UTC (permalink / raw)
To: André Almeida; +Cc: linux-block, osandov, kernel, krisman
On Tue, Oct 29, 2019 at 05:09:40PM -0300, André Almeida wrote:
> 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 981058c..936b0c3 100755
> --- a/check
> +++ b/check
> @@ -635,6 +635,9 @@ 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.
> +
> Miscellaneous:
> -h, --help display this help message and exit"
>
> @@ -650,7 +653,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:c:h' --long 'quick::,exclude:,output:,config:,help' -n "$0" -- "$@"); then
> exit 1
> fi
>
> @@ -659,10 +662,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 +707,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
> + ;;
If the user specifies -c multiple times, it will source each one, not
just the last one. Is that intentional? That might actually be a useful
feature, but we'd want to document it.
> '-h'|'--help')
> usage out
> ;;
> @@ -719,6 +731,13 @@ 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH blktests 0/3] Add --config argument for custom config filenames
2019-10-29 20:09 [PATCH blktests 0/3] Add --config argument for custom config filenames André Almeida
` (2 preceding siblings ...)
2019-10-29 20:09 ` [PATCH blktests 3/3] check: Make "device-only" option a valid option André Almeida
@ 2019-10-30 21:18 ` Omar Sandoval
2019-10-30 22:14 ` André Almeida
3 siblings, 1 reply; 8+ messages in thread
From: Omar Sandoval @ 2019-10-30 21:18 UTC (permalink / raw)
To: André Almeida; +Cc: linux-block, osandov, kernel, krisman
On Tue, Oct 29, 2019 at 05:09:39PM -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é
>
> [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: Add configuration file option
> Documentation: Add information about `--config` argument
> check: Make "device-only" option a valid option
Patches 2 and 3 look good (although a nitpick is that patch 3 could be
first since it's a bug fix that I could take independently of the other
patches). I had one comment on patch 1.
Thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH blktests 1/3] check: Add configuration file option
2019-10-30 21:16 ` Omar Sandoval
@ 2019-10-30 22:05 ` André Almeida
0 siblings, 0 replies; 8+ messages in thread
From: André Almeida @ 2019-10-30 22:05 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, osandov, kernel, krisman
On 10/30/19 6:16 PM, Omar Sandoval wrote:
> On Tue, Oct 29, 2019 at 05:09:40PM -0300, André Almeida wrote:
>> 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 981058c..936b0c3 100755
>> --- a/check
>> +++ b/check
>> @@ -635,6 +635,9 @@ 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.
>> +
>> Miscellaneous:
>> -h, --help display this help message and exit"
>>
>> @@ -650,7 +653,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:c:h' --long 'quick::,exclude:,output:,config:,help' -n "$0" -- "$@"); then
>> exit 1
>> fi
>>
>> @@ -659,10 +662,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 +707,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
>> + ;;
>
> If the user specifies -c multiple times, it will source each one, not
> just the last one. Is that intentional? That might actually be a useful
> feature, but we'd want to document it.
Hmm, good question. Actually, I wasn't thinking about that when I
created this feature. I believe that using all the multiple `-c <file>`
is more useful than just using the last one. I will send a v2
documenting this behavior.
>
>> '-h'|'--help')
>> usage out
>> ;;
>> @@ -719,6 +731,13 @@ 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
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH blktests 0/3] Add --config argument for custom config filenames
2019-10-30 21:18 ` [PATCH blktests 0/3] Add --config argument for custom config filenames Omar Sandoval
@ 2019-10-30 22:14 ` André Almeida
0 siblings, 0 replies; 8+ messages in thread
From: André Almeida @ 2019-10-30 22:14 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, osandov, kernel, krisman
On 10/30/19 6:18 PM, Omar Sandoval wrote:
> On Tue, Oct 29, 2019 at 05:09:39PM -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é
>>
>> [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: Add configuration file option
>> Documentation: Add information about `--config` argument
>> check: Make "device-only" option a valid option
>
> Patches 2 and 3 look good (although a nitpick is that patch 3 could be
> first since it's a bug fix that I could take independently of the other
> patches). I had one comment on patch 1.
>
Nice catch, the order will be fixed at v2 :)
> Thanks!
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-30 22:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-29 20:09 [PATCH blktests 0/3] Add --config argument for custom config filenames André Almeida
2019-10-29 20:09 ` [PATCH blktests 1/3] check: Add configuration file option André Almeida
2019-10-30 21:16 ` Omar Sandoval
2019-10-30 22:05 ` André Almeida
2019-10-29 20:09 ` [PATCH blktests 2/3] Documentation: Add information about `--config` argument André Almeida
2019-10-29 20:09 ` [PATCH blktests 3/3] check: Make "device-only" option a valid option André Almeida
2019-10-30 21:18 ` [PATCH blktests 0/3] Add --config argument for custom config filenames Omar Sandoval
2019-10-30 22:14 ` André Almeida
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).