Re: [PATCH v2] selftest: add tests for debugging testing setup

[Leah Rumancik <leah.rumancik@gmail.com>]

On Thu, Feb 16, 2023 at 01:41:07PM +0800, Zorro Lang wrote:
> On Wed, Feb 15, 2023 at 04:59:04PM -0800, Leah Rumancik wrote:
> > On Wed, Feb 15, 2023 at 07:57:55AM +0200, Amir Goldstein wrote:
> > > On Wed, Feb 15, 2023 at 6:57 AM Zorro Lang <zlang@redhat.com> wrote:
> > > >
> > > > On Tue, Feb 14, 2023 at 01:54:31PM -0800, Leah Rumancik wrote:
> > > > > Many people have developed infrastructure around xfstests. In order to
> > > > > test a setup, it would be helpful to have dummy tests that have
> > > > > consistent test outcomes. Add a new test folder with the following
> > > > > tests:
> > > > >
> > > > > selftest/001 pass
> > > > > selftest/002 fail from output mismatch
> > > > > selftest/003 fail via _fail
> > > > > selftest/004 skip
> > > > > selftest/005 crash
> > > > > selftest/006 hang
> > > > >
> > > > > Also, create two new groups: 'selftest' which includes tests 001-004 and
> > > > > 'dangerous_selftest' which includes tests 005-006. The selftests will
> > > > > run when running all tests but they are not part of the auto group.
> > > > >
> > > > > Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
> > > > > ---
> > > > > v1 -> v2:
> > > > >
> > > > > - change name of folder from 'debug' to 'selftest'
> > > > > - update tags to 'selftest' and 'dangerous_selftest'
> > > > > - add selftest folder to SRC_GROUPS
> > > > > - update copyright tag
> > > > > - add test which _fails
> > > >
> > > > Thanks, this version looks good to me, just each case has a blank line at EOF.
> > > > I'll help to remove those blank lines when I merge this patch.
> > > >
> > > > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > > >
> > > > Thanks,
> > > > Zorro
> > > >
> > > >
> > > > >
> > > > > v1: https://lore.kernel.org/fstests/20230209013143.2586104-1-leah.rumancik@gmail.com/
> > > > >
> > > > >  check                   |  2 +-
> > > > >  doc/group-names.txt     |  2 ++
> > > > >  tests/selftest/001      | 14 ++++++++++++++
> > > > >  tests/selftest/001.out  |  2 ++
> > > > >  tests/selftest/002      | 14 ++++++++++++++
> > > > >  tests/selftest/002.out  |  2 ++
> > > > >  tests/selftest/003      | 16 ++++++++++++++++
> > > > >  tests/selftest/003.out  |  2 ++
> > > > >  tests/selftest/004      | 16 ++++++++++++++++
> > > > >  tests/selftest/004.out  |  2 ++
> > > > >  tests/selftest/005      | 17 +++++++++++++++++
> > > > >  tests/selftest/005.out  |  2 ++
> > > > >  tests/selftest/006      | 20 ++++++++++++++++++++
> > > > >  tests/selftest/006.out  |  2 ++
> > > > >  tests/selftest/Makefile | 24 ++++++++++++++++++++++++
> > > > >  15 files changed, 136 insertions(+), 1 deletion(-)
> > > > >  create mode 100755 tests/selftest/001
> > > > >  create mode 100644 tests/selftest/001.out
> > > > >  create mode 100755 tests/selftest/002
> > > > >  create mode 100644 tests/selftest/002.out
> > > > >  create mode 100755 tests/selftest/003
> > > > >  create mode 100644 tests/selftest/003.out
> > > > >  create mode 100755 tests/selftest/004
> > > > >  create mode 100644 tests/selftest/004.out
> > > > >  create mode 100755 tests/selftest/005
> > > > >  create mode 100644 tests/selftest/005.out
> > > > >  create mode 100755 tests/selftest/006
> > > > >  create mode 100644 tests/selftest/006.out
> > > > >  create mode 100644 tests/selftest/Makefile
> > > > >
> > > > > diff --git a/check b/check
> > > > > index 4827532e..c2065cdc 100755
> > > > > --- a/check
> > > > > +++ b/check
> > > > > @@ -48,7 +48,7 @@ timestamp=${TIMESTAMP:=false}
> > > > >
> > > > >  rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.xlist $tmp.report.* $tmp.arglist
> > > > >
> > > > > -SRC_GROUPS="generic shared"
> > > > > +SRC_GROUPS="generic shared selftest"
> > > 
> > > I know this is a bit of bikeshedding,
> > > but is it really needed to add selftest/* to SRC_GROUPS and then filter it out
> > > by group?
> > > 
> > > Seems to me that it would be simpler to not add any new groups and use
> > > ./check -g selftest/quick -g selftest/dangerous
> > > to run the tests in the selftests subdir.
> > > 
> > > Just to be clear, the syntax above already works, nothing new to implement.
> > > 
> > > I do not object to adding the selftest groups if others like it better this way.
> > 
> > I think it behaves more as expected when it is a part of SRC_GROUPS, but
> > I will probably run these by specifying specific tests - so I don't
> > care much about which groups are used / if it is in SRC_GROUPS or not,
> > and I am happy to change it if others agree.
> 
> I agree with the suggestion from Amir. The "selftest" should be an independent
> test group only be run when a tester intend to run it.


When you say "selftest" should be an independent test group, do you mean
we should keep the "selftest" and "dangerous_selftest" groups but just
remove the "selftest" folder from SRC_GROUPS? I think Amir was suggesting
to completely remove the test groups "selftest" and
"dangerous_selftest" as well as removing the folder from SRC_GROUPS.

- Leah

> 
> Thanks,
> Zorro
> 
> > 
> > > 
> > > It would also be nice to drop a line in README about running selftests.
> > 
> > Sure, I can do this.
> > 
> > > 
> > > Thanks,
> > > Amir.
> > 
>