On Wed, Sep 27, 2023 at 08:30:05AM +0200, David Marchand wrote:

On Tue, Sep 26, 2023 at 5:01 PM Aaron Conole [off-list ref] wrote:

quoted

David Marchand [off-list ref] writes:

quoted

On Tue, Sep 19, 2023 at 10:36 AM Bruce Richardson
[off-list ref] wrote:

quoted

quoted

quoted

To help ensure that we don't have "orphaned" tests not in any test
suites we can add the following checks:

* In developer-mode builds, emit a warning for each test defined using
  REGISTER_TEST_COMMAND
* In checkpatches, add a check to prevent the addition of new tests
  using the REGISTER_TEST_COMMAND macro

Bruce Richardson (2):
  app/test: emit warning for tests not in a test suite
  devtools: check for tests added without a test suite

 app/test/suites/meson.build   | 13 ++++++++++++-
 buildtools/get-test-suites.py | 12 +++++++++---
 devtools/checkpatches.sh      |  8 ++++++++
 3 files changed, 29 insertions(+), 4 deletions(-)

The "non_suite_tests" testsuite returned by
buildtools/get-test-suites.py is a bit strange, as it is not a
testsuite from meson pov.

Yeah, it is a bit strange, and I'm open to new ideas on other solutions. I
did it that way to avoid having yet another script to scan the files - I
figured it was faster (in terms of runtime, not dev time) to do the

I had figured it was "faster dev time" that won :-).
I am fine with it, I don't expect more complications in this area in the future.

quoted

scanning when the files are already being opened and processed by this one.

Of course, if we can get the un-suitened [:-)] test cases down to zero, we
can theoretically drop this check in future, and  just use the checkpatch
one.

Well, that's still a question that nobody seems to comment on.

What should we do with tests that don't enter one of those testsuites,
and are not invoked by the CI?

Though we may be removing some level of coverage, I am for
cleaning/unused dead code.

I guess it does require actually looking at these tests and classifying
them to either put them into the proper suites.  As of now, we aren't
really removing coverage if they aren't being run - but are any
maintainers or developers actually running them?

Could we go a step further than Bruce runtime warning (which is at the
meson level and does not impact running the test)?
Perhaps have those orphaned tests fail unless their test names are
provided in a env variable like
DPDK_TRUST_ME_I_WILL_SUBMIT_A_PATCH_FOR_THIS_TEST (naming is hard
;-))?

With a systematic failure, there is less chance that
developers/maintainers miss the situation.
If those developers/maintainers simply waive the warning with the env
variable and don't send a patch, well.. too bad.

After a release or two, if we don't hear from anyone, we can start
removing the unused one.

I think that seems a littel severe at this point.

The one gap we have right now, as far as I can see, is actually explaining
what the various suite types are, so that developers can choose the right
one for their test(s).  We may even need a couple more if some tests do not
fit into the existing categories. Once that is done, we should then start
looking for tests that are obsolete and can be removed.

If we do already have documentation on the various suites and how to use
them, apologies for my ignorance, and perhaps someone could post a link
here.

/Bruce