Re: [PATCH v2 1/2] selftests/mm: Reject invalid test selections before running tests

["David Hildenbrand (Arm)" <david@kernel.org>]

On 9/8/26 11:55, Tianyi Chen wrote:
> getopts reports unknown options and missing arguments, but run_vmtests.sh
> ignores its error result and continues with test setup. An empty -t
> argument also falls back to the default selection, while unknown category
> names can silently select no tests and still reach setup code.
> 
> Exit on getopts errors and validate category names against the existing
> list in usage() before any test setup. Reject empty and whitespace-only
> selections, and normalize category separators so validation and execution
> agree. Keep the default selection for an unset or empty VM_SELFTEST_ITEMS
> environment variable, and preserve the existing standalone default value.
> 
> Fixes: 85463321e726 ("selftests/vm: enable running select groups of tests")
> Assisted-by: Codex:GPT-6
> Signed-off-by: Tianyi Chen <hi@tychen.cc>
> ---
>  tools/testing/selftests/mm/run_vmtests.sh | 27 +++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index d09f9f6a384..836c7454cba 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -96,26 +96,45 @@ separated by spaces:
>  
>  example: ./run_vmtests.sh -t "hmm mmap ksm"
>  EOF
> -	exit 0
>  }
>  
>  RUN_ALL=false
>  RUN_DESTRUCTIVE=false
>  TAP_PREFIX="# "
>  
> +# An unset or empty environment selection keeps the default behavior.
> +VM_SELFTEST_ITEMS=${VM_SELFTEST_ITEMS:-default}

Who would set it at this point?

Can't this just be

VM_SELFTEST_ITEMS="default"

> +
>  while getopts "aht:nd" OPT; do
>  	case ${OPT} in
>  		"a") RUN_ALL=true ;;
> -		"h") usage ;;
> +		"h") usage; exit 0 ;;
>  		"t") VM_SELFTEST_ITEMS=${OPTARG} ;;
>  		"n") TAP_PREFIX= ;;
>  		"d") RUN_DESTRUCTIVE=true ;;
> +		"?") exit 1 ;;
>  	esac
>  done
>  shift $((OPTIND -1))
>  
> -# default behavior: run all tests
> -VM_SELFTEST_ITEMS=${VM_SELFTEST_ITEMS:-default}
> +# Normalize whitespace so validation and test_selected() use the same names.
> +read -r -a selected_categories <<< "${VM_SELFTEST_ITEMS//$'\n'/ }"
> +VM_SELFTEST_ITEMS="${selected_categories[*]}"
> +if [ -z "$VM_SELFTEST_ITEMS" ]; then
> +	echo "No test categories specified" >&2
> +	exit 1
> +fi
> +
> +if [ "$VM_SELFTEST_ITEMS" != "default" ]; then
> +	# Keep the documented category list as the source of valid names.

That makes sense to me.

It's interesting what would happen when trying to run a test that does not even
exist for the architecture. Getting some feedback that the test was skipped
would be nice. But that's something for another day :)

-- 
Cheers,

David