| Message ID | 20240411111228.2290407-2-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | support test case repeat by different conditions | expand |
On 11/04/24 08:12PM, Shin'ichiro Kawasaki wrote: >The function _run_test() is rather complex and has deep nests. Before >modifying it for repeated test case runs, simplify it. Factor out some >part of the function to the new functions _check_and_call_test() and >_check_and_call_test_device(). > >Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> >--- > check | 90 +++++++++++++++++++++++++++++++++++------------------------ > 1 file changed, 53 insertions(+), 37 deletions(-) > >diff --git a/check b/check >index 55871b0..b1f5212 100755 >--- a/check >+++ b/check >@@ -463,6 +463,56 @@ _unload_modules() { > unset MODULES_TO_UNLOAD > } > >+_check_and_call_test() { ret should be declared ret as local ? >+ if declare -fF requires >/dev/null; then >+ requires >+ fi >+ >+ RESULTS_DIR="$OUTPUT/nodev" >+ _call_test test >+ ret=$? >+ if (( RUN_ZONED_TESTS && CAN_BE_ZONED )); then >+ RESULTS_DIR="$OUTPUT/nodev_zoned" >+ RUN_FOR_ZONED=1 >+ _call_test test >+ ret=$(( ret || $? )) >+ fi >+ >+ return $ret >+} >+ >+_check_and_call_test_device() { >+ local unset_skip_reason Same here, ret should declared be local ? Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>
On Apr 11, 2024 / 19:04, Nitesh Shetty wrote: > On 11/04/24 08:12PM, Shin'ichiro Kawasaki wrote: > > The function _run_test() is rather complex and has deep nests. Before > > modifying it for repeated test case runs, simplify it. Factor out some > > part of the function to the new functions _check_and_call_test() and > > _check_and_call_test_device(). > > > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > --- > > check | 90 +++++++++++++++++++++++++++++++++++------------------------ > > 1 file changed, 53 insertions(+), 37 deletions(-) > > > > diff --git a/check b/check > > index 55871b0..b1f5212 100755 > > --- a/check > > +++ b/check > > @@ -463,6 +463,56 @@ _unload_modules() { > > unset MODULES_TO_UNLOAD > > } > > > > +_check_and_call_test() { > > ret should be declared ret as local ? Yes, will do so in v2. > > > + if declare -fF requires >/dev/null; then > > + requires > > + fi > > + > > + RESULTS_DIR="$OUTPUT/nodev" > > + _call_test test > > + ret=$? > > + if (( RUN_ZONED_TESTS && CAN_BE_ZONED )); then > > + RESULTS_DIR="$OUTPUT/nodev_zoned" > > + RUN_FOR_ZONED=1 > > + _call_test test > > + ret=$(( ret || $? )) > > + fi > > + > > + return $ret > > +} > > + > > +_check_and_call_test_device() { > > + local unset_skip_reason > > Same here, ret should declared be local ? Yes, and thanks for the review :) > > Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>
diff --git a/check b/check index 55871b0..b1f5212 100755 --- a/check +++ b/check @@ -463,6 +463,56 @@ _unload_modules() { unset MODULES_TO_UNLOAD } +_check_and_call_test() { + if declare -fF requires >/dev/null; then + requires + fi + + RESULTS_DIR="$OUTPUT/nodev" + _call_test test + ret=$? + if (( RUN_ZONED_TESTS && CAN_BE_ZONED )); then + RESULTS_DIR="$OUTPUT/nodev_zoned" + RUN_FOR_ZONED=1 + _call_test test + ret=$(( ret || $? )) + fi + + return $ret +} + +_check_and_call_test_device() { + local unset_skip_reason + + if declare -fF requires >/dev/null; then + requires + fi + + for TEST_DEV in "${TEST_DEVS[@]}"; do + TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}" + TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}" + + unset_skip_reason=0 + if [[ ! -v SKIP_REASONS ]]; then + unset_skip_reason=1 + if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then + SKIP_REASONS+=("${TEST_DEV} is a zoned block device") + elif declare -fF device_requires >/dev/null; then + device_requires + fi + fi + RESULTS_DIR="$OUTPUT/$(basename "$TEST_DEV")" + if ! _call_test test_device; then + ret=1 + fi + if (( unset_skip_reason )); then + unset SKIP_REASONS + fi + done + + return $ret +} + _run_test() { TEST_NAME="$1" CAN_BE_ZONED=0 @@ -482,19 +532,8 @@ _run_test() { . "tests/${TEST_NAME}" if declare -fF test >/dev/null; then - if declare -fF requires >/dev/null; then - requires - fi - - RESULTS_DIR="$OUTPUT/nodev" - _call_test test + _check_and_call_test ret=$? - if (( RUN_ZONED_TESTS && CAN_BE_ZONED )); then - RESULTS_DIR="$OUTPUT/nodev_zoned" - RUN_FOR_ZONED=1 - _call_test test - ret=$(( ret || $? )) - fi else if [[ ${#TEST_DEVS[@]} -eq 0 ]] && \ declare -fF fallback_device >/dev/null; then @@ -516,31 +555,8 @@ _run_test() { return 0 fi - if declare -fF requires >/dev/null; then - requires - fi - - for TEST_DEV in "${TEST_DEVS[@]}"; do - TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}" - TEST_DEV_PART_SYSFS="${TEST_DEV_PART_SYSFS_DIRS["$TEST_DEV"]}" - - local unset_skip_reason=0 - if [[ ! -v SKIP_REASONS ]]; then - unset_skip_reason=1 - if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then - SKIP_REASONS+=("${TEST_DEV} is a zoned block device") - elif declare -fF device_requires >/dev/null; then - device_requires - fi - fi - RESULTS_DIR="$OUTPUT/$(basename "$TEST_DEV")" - if ! _call_test test_device; then - ret=1 - fi - if (( unset_skip_reason )); then - unset SKIP_REASONS - fi - done + _check_and_call_test_device + ret=$? if (( FALLBACK_DEVICE )); then cleanup_fallback_device
The function _run_test() is rather complex and has deep nests. Before modifying it for repeated test case runs, simplify it. Factor out some part of the function to the new functions _check_and_call_test() and _check_and_call_test_device(). Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> --- check | 90 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 37 deletions(-)