Re: [dpdk-dev] [PATCH] doc/guides: add details for new test structure

["Power, Ciara" <ciara.power@intel.com>]

Hi Aaron,


>-----Original Message-----
>From: Aaron Conole <aconole@redhat.com>
>Sent: Monday 17 May 2021 19:02
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Doherty,
>Declan <declan.doherty@intel.com>
>Subject: Re: [PATCH] doc/guides: add details for new test structure
>
>Ciara Power <ciara.power@intel.com> writes:
>
>> The testing guide is now updated to include details about using
>> sub-testsuites. Some example code is given to demonstrate how they can
>> be used.
>>
>> A note is also added to highlight the need for using vdev EAL args
>> when running cryptodev tests.
>>
>> Depends-on: patch-88751 ("guides: add a guide for developing unit
>> tests")
>>
>> Signed-off-by: Ciara Power <ciara.power@intel.com>
>> ---
>>  doc/guides/contributing/testing.rst | 89
>> ++++++++++++++++++++++++++++-
>>  1 file changed, 87 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/contributing/testing.rst
>> b/doc/guides/contributing/testing.rst
>> index 0757d71ad0..1188d63a97 100644
>> --- a/doc/guides/contributing/testing.rst
>> +++ b/doc/guides/contributing/testing.rst
>> @@ -114,13 +114,21 @@ for interacting with the test harness:
>>
>>    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, tear down, and vector of unit test
>> -     cases.
>> +     a test suite name, setup, tear down, a pointer to a list of
>> +     sub-testsuites, and vector of unit test cases.
>>
>>  Each test suite has a setup and tear down 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 tear down.
>>
>> +Each testsuite may use a nested list of sub-testsuites, which are
>> +iterated by the unit_test_suite_runner.
>> +This support allows for better granularity when designing test suites.
>> +The sub-testsuites list can also be used in parallel with the vector
>> +of testcases, in this case the testcases will be run, and then each
>> +sub-testsuite is executed. To see an example of a testsuite using
>> +sub-testsuites, see *app/test/test_cryptodev.c*.
>> +
>>  Test cases are added to the `.unit_test_cases` element of the unit
>> test suite structure.  Ex:
>>
>> @@ -165,6 +173,70 @@ test suite structure.  Ex:
>>  The above code block is a small example that can be used to create a
>> complete test suite with test case.
>>
>> +Sub-testsuites can be added to the `.unit_test_suites` element of the
>> +unit test suite structure.  Ex:
>> +
>> +.. code-block:: c
>> +   :linenos:
>> +
>> +   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_parent_testsuite = {
>> +          .suite_name = "EXAMPLE PARENT TEST SUITE",
>> +          .setup = testsuite_setup,
>> +          .teardown = testsuite_teardown,
>> +          .unit_test_cases = {TEST_CASES_END()}
>> +   };
>> +
>> +   static int sub_testsuite_setup(void) { return TEST_SUCCESS; }
>> +   static void sub_testsuite_teardown(void) { }
>> +
>> +   static struct unit_test_suite example_sub_testsuite = {
>> +          .suite_name = "EXAMPLE SUB TEST SUITE",
>> +          .setup = sub_testsuite_setup,
>> +          .teardown = sub_testsuite_teardown,
>> +          .unit_test_cases = {
>> +               TEST_CASE_ST(ut_setup, ut_teardown, test_case_first),
>> +
>> +               TEST_CASES_END(), /**< NULL terminate unit test array */
>> +          },
>> +   };
>> +
>> +   static struct unit_test_suite end_testsuite = {
>> +          .suite_name = NULL,
>> +          .setup = NULL,
>> +          .teardown = NULL,
>> +          .unit_test_suites = NULL
>> +   };
>> +
>> +   static int example_tests()
>> +   {
>> +       uint8_t ret, i = 0;
>> +       struct unit_test_suite *sub_suites[] = {
>> +              &example_sub_testsuite,
>> +              &end_testsuite /**< NULL test suite to indicate end of list */
>> +        };
>> +
>> +       example_parent_testsuite.unit_test_suites =
>> +               malloc(sizeof(struct unit_test_suite *) *
>> + RTE_DIM(sub_suites));
>> +
>> +       for (i = 0; i < RTE_DIM(sub_suites); i++)
>> +           example_parent_testsuite.unit_test_suites[i] =
>> + sub_suites[i];
>> +
>> +       ret = unit_test_suite_runner(&example_parent_testsuite);
>> +       free(example_parent_testsuite.unit_test_suites);
>> +
>> +       return ret;
>> +   }
>> +
>> +   REGISTER_TEST_COMMAND(example_autotest, example_tests);
>> +
>>
>>  Designing a test
>>  ----------------
>> @@ -241,3 +313,16 @@ 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.
>> +
>> +
>> +Running Cryptodev Tests
>> +-----------------------
>> +
>> +When running cryptodev tests, the user must create any required
>> +virtual device via EAL args, as this is not automatically done by the test.
>
>How does this differ from just running the tests?  Will I get 'SKIP' or 'FAIL' if I
>don't do this?  Should the cryptodev tests stay as part of the driver test suite
>or be moved out?
>

Before the refactor, the vdev creation was done by the test app when needed.
For example, with DPDK_TEST set to "cryptodev_aesni_mb_autotest", which needs a crypto_aesni_mb vdev, the user would just need to run:
./dpdk-test

But now with this refactor, this vdev isn't created by the test app any longer. For the same autotest, the user must now run:
./dpdk-test --vdev crypto_aesni_mb

Without passing the vdev argument, the test is skipped with log: "USER1: No crypto_aesni_mb devices found?"


With regards the driver test suite, if the suite is run without any extra args, for example:
meson test --suite=DPDK:driver-tests -Cbuild
The vdevs will not be created, and the autotests that don't have the vdev requirements met will be skipped.

But we can pass extra test args to create the vdevs we want to test:
meson test --suite=DPDK:driver-tests -Cbuild --test-args="--vdev crypto_aesni_mb --vdev crypto_aesni_gcm"

I think it is still ok to include these in the driver suite, and allow the user to choose the vdevs they need and can support on their system.

Thanks,
Ciara


>> +
>> +.. note::
>> +
>> +   The cryptodev_scheduler_autotest is the only exception to this.
>> +   This vdev will be created automatically by the test app,
>> +   as it requires a more complex setup than other vdevs.