Re: [RFC PATCH v1.1 06/11] selftests/damon/sysfs.sh: test dests dir

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `test_dests()` function is never invoked, and its helper `test_dest()` omits a required argument, causing the test to be both unexercised and structurally broken.
--

commit 53579293376a979c6b9c9914d1d77ffa57418d36
Author: SeongJae Park <sj@kernel.org>

selftests/damon/sysfs.sh: test dests dir

DAMON selftest interface essential file operations test (sysfs.sh) is
not testing DAMOS dests/ directory. This commit adds a test for the
dests directory.

> diff --git a/tools/testing/selftests/damon/sysfs.sh b/tools/testing/selftests/damon/sysfs.sh
> index 07a33995be852..ffa8413b5ab3d 100755
> --- a/tools/testing/selftests/damon/sysfs.sh
> +++ b/tools/testing/selftests/damon/sysfs.sh
> @@ -99,6 +99,29 @@ test_stats()
>  	done
>  }
>  
> +test_dest()
> +{
> +	dest_dir=$1
> +	ensure_file "$dest_dir/id" "exist"
> +	ensure_file "$dest_dir/weight" "exist"

[Severity: Medium]
Does ensure_file() require a third argument for the expected permission?
Without it, the underlying check might evaluate the file's permission against
an empty string and unconditionally fail the test.

> +}
> +
> +test_dests()
> +{

[Severity: Medium]
Is test_dests() ever invoked? It looks like this function is defined here
but remains unreferenced in the script. Should it be called from
test_scheme() to verify the directory under each scheme?

> +	dests_dir=$1
> +	ensure_file "$dests_dir/nr_dests" "exist" "600"
> +	ensure_write_succ "$dests_dir/nr_dests" "1" "valid input"
> +	test_dest "$dests_dir/0"

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625050756.91115-1-sj@kernel.org?part=6