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 <[email protected]>
> ---
> 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
