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

[SeongJae Park <sj@kernel.org>]

On Thu, 25 Jun 2026 05:14:49 +0000 sashiko-bot@kernel.org wrote:

> 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?

Good findings!  I will fix those in the next revision.


Thanks,
SJ

[...]