[RFC] doc: add a guide to describe developing tests

Message ID 20210127151618.632975-1-aconole@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [RFC] doc: add a guide to describe developing tests |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Aaron Conole Jan. 27, 2021, 3:16 p.m. UTC
  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>
---
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
  

Comments

Bruce Richardson Jan. 27, 2021, 5:29 p.m. UTC | #1
On Wed, Jan 27, 2021 at 10:16:18AM -0500, Aaron Conole wrote:
> 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
> ---
> 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>
> +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?

> +
<snip>
> +
> +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.
  
Aaron Conole Jan. 27, 2021, 6:28 p.m. UTC | #2
Hi Bruce,

Thanks for the review!  Responses below.

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Wed, Jan 27, 2021 at 10:16:18AM -0500, Aaron Conole wrote:
>> 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
>> ---
>> 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>
>> +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.

>> +
> <snip>
>> +
>> +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.
  

Patch

diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst
index 2fefd91931..41909d949b 100644
--- a/doc/guides/contributing/index.rst
+++ b/doc/guides/contributing/index.rst
@@ -14,6 +14,7 @@  Contributor's Guidelines
     abi_versioning
     documentation
     patches
+    testing
     vulnerability
     stable
     cheatsheet
diff --git a/doc/guides/contributing/testing.rst b/doc/guides/contributing/testing.rst
new file mode 100644
index 0000000000..2356ff11cc
--- /dev/null
+++ b/doc/guides/contributing/testing.rst
@@ -0,0 +1,200 @@ 
+..  SPDX-License-Identifier: BSD-3-Clause
+    Copyright 2018 The DPDK contributors
+
+.. _testing_guidelines:
+
+DPDK Testing Guidelines
+=======================
+
+This document outlines the guidelines for running and adding new
+tests to the in-tree DPDK test suites.
+
+The DPDK test suite model is loosely based on the xunit model, where
+tests are grouped into test suites, and suites are run by runners.
+For a basic overview, see the basic Wikipedia article on xunit:
+`xUnit - Wikipedia <https://en.wikipedia.org/wiki/XUnit>`_.
+
+
+Running a test
+--------------
+
+DPDK tests are run via the main test runniner, the `dpdk-test` app.
+The `dpdk-test` app is a command-line interface that facilitates
+running various tests or test suites.
+
+There are two modes of operation.  The first mode is as an interactive
+command shell that allows launching specific test suites.  This is
+the default operating mode of `dpdk-test` and can be done by::
+
+  $ ./build/app/test/dpdk-test --dpdk-options-here
+  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>>
+
+At the prompt, simply type the name of the test suite you wish to run
+and it will execute.
+
+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').
+
+
+Running all tests
+-----------------
+
+In order to allow developers to quickly execute all the standard
+internal tests without needing to remember or look up each test suite
+name, the build system includes a standard way of executing the
+default test suites.  After building via `ninja`, the ``meson test``
+command will execute the standard tests and report errors.
+
+There are four groups of default test suites.  The first group is
+the **fast** test suite, which is the largest group of test cases.
+These are the bulk of the unit tests to validate functional blocks.
+The second group is the **perf** tests.  These test suites can take
+longer to run and do performance evaluations.  The third group is
+the **driver** test suite, which is mostly for special hardware
+related testing (such as `cryptodev`).  The last group are the
+**debug** tests.  These mostly are used to dump system information.
+
+The suites can be selected by adding the ``--suite`` option to the
+``meson test`` command.  Ex: ``meson test --suite fast-tests``
+
+
+Adding test suites
+------------------
+
+To add a testsuite to the DPDK test application, create a new test
+file for that suite (ex: see *app/test/test_version.c* for the
+``version_autotest`` test suite).  There are two useful things:
+
+  1. REGISTER_TEST_COMMAND(command_name, function_to_execute)
+     Registers a test command with the name `command_name` and which
+     runs the function `function_to_execute` when `command_name` is
+     invoked.
+
+  2. unit_test_suite_runner(struct unit_test_suite \*)
+     Returns a runner for a full test suite object, which contains
+     a test suite name, setup, teardown, and vector of unit test
+     cases.
+
+Each test suite has a setup and teardown function that runs at the
+beginning and end of the test suite execution.  Each unit test has
+a similar function for test case setup and teardown.
+
+Adding test cases is controlled via the `.unit_test_cases` element
+of the unit test suite.  Ex:
+
+.. code-block:: c
+   :linenos:
+
+   #include <time.h>
+
+   #include <rte_common.h>
+   #include <rte_cycles.h>
+   #include <rte_hexdump.h>
+   #include <rte_random.h>
+
+   #include "test.h"
+
+   static int testsuite_setup(void) { return TEST_SUCCESS; }
+   static void testsuite_teardown(void) { }
+
+   static int ut_setup(void) { return TEST_SUCCESS; }
+   static void ut_teardown(void) { }
+
+   static int test_case_first(void) { return TEST_SUCCESS; }
+
+   static struct unit_test_suite example_testsuite = {
+          .suite_name = "EXAMPLE TEST SUITE",
+          .setup = testsuite_setup,
+          .teardown = testsuite_teardown,
+          .unit_test_cases = {
+               TEST_CASE_ST(ut_setup, ut_teardown, test_case_first),
+
+               TEST_CASES_END(), /**< NULL terminate unit test array */
+          },
+   };
+
+   static int example_tests()
+   {
+       return unit_test_suite_runner(&example_testsuite);
+   }
+
+   REGISTER_TEST_COMMAND(example_autotest, example_tests);
+
+The above code block is a small example that can be used to create a
+complete test suite with test case.
+
+
+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.
+
+
+Adding a suite to the default
+-----------------------------
+
+Adding to one of the default tests involves editing the appropriate
+meson build file `app/test/meson.build` and adding the command to
+the correct test suite class.  Once added, the new test suite will
+be run as part of the appropriate class (fast, perf, driver, etc.).
+
+Some of these default test suites are run during continuous integration
+tests, making regression checking automatic for new patches submitted
+to the project.