Re: [PATCH v2 5/9] test_sysctl: add generic script to expand on tests

[Luis R. Rodriguez]

On Mon, Feb 13, 2017 at 12:30:22PM -0800, Kees Cook wrote:
> On Fri, Feb 10, 2017 at 4:36 PM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> > This adds a generic script to let us more easily add more tests
> > cases. Since we really have only two types of tests cases just
> > fold them into the one file. Each test unit is now identified
> > into its separate function:
> >
> >   # ./sysctl.sh -l
> > Test ID list:
> >
> > TEST_ID x NUM_TEST
> > TEST_ID:   Test ID
> > NUM_TESTS: Number of recommended times to run the test
> >
> > 0001 x 1 - tests proc_dointvec_minmax()
> > 0002 x 1 - tests proc_dostring()
> >
> > For now we start off with what we had before, and run only each test once.
> > We can now watch a test case until it fails:
> >
> > ./sysctl.sh -w 0002
> >
> > We can also run a test case x number of times, say we want to run
> > a test case 100 times:
> >
> > ./sysctl.sh -c 0001 100
> >
> > To run a test case only once, for example:
> >
> > ./sysctl.sh -s 0002
> >
> > The default settings are specified at the top of sysctl.sh.
> >
> > Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> 
> I'm not a fan of this: it consolidates tests when it's not needed 

Tests can easily be split off even with the above syntax, right now its all
just stuffed in one file but the syntax does not impede this to change.

> and creates a test running infrastructure at the wrong level of
> abstraction.

In lieu of an existing abstraction layer which provides this having each script
*for now* do what it wishes seems fair. To create an generic test abstraction
layer we need to study each test case, and I'm afraid I can't do that at this
time. But I do by now have about 3 test drivers pending upstream which share
similar testing taste, so I can later try to groom a generic infrastructure for
what I want but for now I can't find the issue with having each test script
have what it needs.

> I'd like to see individual tests that are one-off runnable.

The above allows for this: './sysctl.sh -s 0002' will run the test case 0002
once.

> Whatever consumes the tools/testing/selftests/ tree is what
> should be doing the -w, -c, etc style options.

In the end I agree, but I also believe in evolving this.

If you feel strongly this needs to be generalized at the right layer from the
start I'm afraid I'll just have to drop these tests as I just don't have the
time to address expanding the selftest infrastructure with a generic solution
which covers all that I added, I expect this to be quite a bit of work.

  Luis