Re: [dpdk-dev] [RFC] doc: add a guide to describe developing tests

Hi Bruce,

Thanks for the review!  Responses below.

Bruce Richardson [off-list ref] writes:

On Wed, Jan 27, 2021 at 10:16:18AM -0500, Aaron Conole wrote:

quoted

The DPDK testing infrastructure includes a comprehensive set of
libraries, utilities, and CI integrations for developers to test
their code changes.  This isn't well documented, however.

Document the basics for adding a test suite to the infrastructure
and enabling that test suite for continuous integration platforms
so that newer developers can understand how to develop test suites
and test cases.

Signed-off-by: Aaron Conole <aconole@redhat.com>

Thanks for starting this Aaron, this doc is well needed. Couple of comments
below.

/Bruce

quoted

---
Submitting as RFC as I'm not sure if this should include making
changes to the github actions or travis yml, or the linux-build
ci script.

 doc/guides/contributing/index.rst   |   1 +
 doc/guides/contributing/testing.rst | 200 ++++++++++++++++++++++++++++
 2 files changed, 201 insertions(+)
 create mode 100644 doc/guides/contributing/testing.rst

<snip>

quoted

+The second form is useful for a scripting environment, and is used by
+the DPDK meson build system.  This mode is invoked by assigning a
+specific test suite name to the environment variable `DPDK_TEST`
+before invoking the `dpdk-test` command, such as::
+
+  $ DPDK_TEST=version_autotest ./build/app/test/dpdk-test --no-huge
+  EAL: Detected 4 lcore(s)
+  EAL: Detected 1 NUMA nodes
+ EAL: Static memory layout is selected, amount of reserved memory
can be adjusted with -m or --socket-mem
+  EAL: Multi-process socket /run/user/26934/dpdk/rte/mp_socket
+  EAL: Selected IOVA mode 'VA'
+  EAL: Probing VFIO support...
+  EAL: PCI device 0000:00:1f.6 on NUMA socket -1
+  EAL:   Invalid NUMA socket, default to 0
+  EAL:   probe driver: 8086:15d7 net_e1000_em
+  APP: HPET is not enabled, using TSC as default timer
+  RTE>>version_autotest
+  Version string: 'DPDK 20.02.0-rc0'
+  Test OK
+  RTE>>$
+
+The above shows running a specific test case.  On success, the return
+code will be '0', otherwise it will be set to some error value (such
+as '255').
+

This reminds me that I have patch almost ready to submit to extend this
support to allow passing the test name - or even multiple test names - on
the commandline, not just through the environment. Will upstream it
shortly, I hope. I think command-line is more usable for folks than the
environment. Thoughts?

I agree, such a change would be really useful as a developer.  I have no
preference when it comes to the scripting side though (which is what
this section is about).  Either we set env in the script, or we pass as
command line.

quoted

+

<snip>

quoted

+
+Designing a test
+----------------
+
+Test cases have multiple ways of indicating an error has occurred,
+in order to reflect failure state back to the runner.  Using the
+various methods of indicating errors can assist in not only validating
+the requisite functionality is working, but also to help debug when
+a change in environment or code has caused things to go wrong.
+
+The first way to indicate a generic error is by returning a test
+result failure, using the *TEST_FAILED* error code.  This is the most
+basic way of indicating that an error has occurred in a test routine.
+It isn't very informative to the user, so it should really be used in
+cases where the test has catastrophically failed.
+
+The preferred method of indicating an error is via the
+`RTE_TEST_ASSERT` family of macros, which will immediately return
+*TEST_FAILED* error condition, but will also log details about the
+failure.  The basic form is:
+
+.. code-block:: c
+
+   RTE_TEST_ASSERT(cond, msg, ...)
+
+In the above macro, *cond* is the condition to evaluate to **true**.
+Any generic condition can go here.  The *msg* parameter will be a
+message to display if *cond* evaluates to **false**.  Some specialized
+macros already exist.  See `lib/librte_eal/include/rte_test.h` for
+a list of pre-build test assertions.
+

Maybe also mention TEST_SKIPPED return value.

Okay, I will add some description about it.