[dpdk-dev] [PATCH 1/6] app/test: refactor of unit test suite runner
Aaron Conole
aconole at redhat.com
Wed Mar 31 16:42:26 CEST 2021
Ciara Power <ciara.power at intel.com> writes:
> Some small changes were made to the unit test suite runner for
> readability and to enable reuse of some of the function in a later patch.
>
> On test suite setup skip/fail, the loop to count testcases as
> skipped/failed has been moved to another function.
> This will allow for recursion in a later patch when nested sub-testsuites
> are used.
>
> The unit test suite runner accessed the list of testcases in the suite
> structure every time the testcase was used. This is now replaced by a
> testcase variable which improves readability.
>
> The summary output now prints the suite name, this will be useful later
> when multiple nested sub-testsuites are being run.
>
> Signed-off-by: Ciara Power <ciara.power at intel.com>
> ---
I see lots of open coded loops in here. Does it make sense to have
something like:
#define FOR_EACH_SUITE_TESTCASE(iter, suite, case) \
for (iter = 0, case = suite->unit_test_case[0]; \
suite->unit_test_cases[iter]; \
iter++, case = suite->unit_test_cases[iter])
Then in code we can do:
struct unit_test_case tc;
size_t total;
FOR_EACH_SUITE_TESTCASE(total, suite, tc) {
... check something ...
}
It would help reading the patch.
> app/test/test.c | 51 ++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/app/test/test.c b/app/test/test.c
> index 624dd48042..72768c8854 100644
> --- a/app/test/test.c
> +++ b/app/test/test.c
> @@ -207,6 +207,23 @@ main(int argc, char **argv)
> return ret;
> }
>
> +static void
> +unit_test_suite_count_tcs_on_setup_fail(struct unit_test_suite *suite,
> + int test_success, unsigned int *total, unsigned int *skipped,
> + unsigned int *failed)
> +{
> + struct unit_test_case tc;
> +
> + tc = suite->unit_test_cases[*total];
> + while (tc.testcase) {
> + if (!tc.enabled || test_success == TEST_SKIPPED)
> + (*skipped)++;
> + else
> + (*failed)++;
> + (*total)++;
> + tc = suite->unit_test_cases[*total];
> + }
> +}
>
> int
> unit_test_suite_runner(struct unit_test_suite *suite)
> @@ -215,6 +232,7 @@ unit_test_suite_runner(struct unit_test_suite *suite)
> unsigned int total = 0, executed = 0, skipped = 0;
> unsigned int succeeded = 0, failed = 0, unsupported = 0;
> const char *status;
> + struct unit_test_case tc;
>
> if (suite->suite_name) {
> printf(" + ------------------------------------------------------- +\n");
> @@ -228,38 +246,35 @@ unit_test_suite_runner(struct unit_test_suite *suite)
> * setup did not pass, so count all enabled tests and
> * mark them as failed/skipped
> */
> - while (suite->unit_test_cases[total].testcase) {
> - if (!suite->unit_test_cases[total].enabled ||
> - test_success == TEST_SKIPPED)
> - skipped++;
> - else
> - failed++;
> - total++;
> - }
> + unit_test_suite_count_tcs_on_setup_fail(suite,
> + test_success, &total,
> + &skipped, &failed);
> goto suite_summary;
> }
> }
>
> printf(" + ------------------------------------------------------- +\n");
>
> - while (suite->unit_test_cases[total].testcase) {
> - if (!suite->unit_test_cases[total].enabled) {
> + tc = suite->unit_test_cases[total];
> + while (tc.testcase) {
> + if (!tc.enabled) {
> skipped++;
> total++;
> + tc = suite->unit_test_cases[total];
> continue;
> } else {
> executed++;
> }
>
> /* run test case setup */
> - if (suite->unit_test_cases[total].setup)
> - test_success = suite->unit_test_cases[total].setup();
> + if (tc.setup)
> + test_success = tc.setup();
> else
> test_success = TEST_SUCCESS;
>
> if (test_success == TEST_SUCCESS) {
> /* run the test case */
> - test_success = suite->unit_test_cases[total].testcase();
> + test_success = tc.testcase();
> if (test_success == TEST_SUCCESS)
> succeeded++;
> else if (test_success == TEST_SKIPPED)
> @@ -275,8 +290,8 @@ unit_test_suite_runner(struct unit_test_suite *suite)
> }
>
> /* run the test case teardown */
> - if (suite->unit_test_cases[total].teardown)
> - suite->unit_test_cases[total].teardown();
> + if (tc.teardown)
> + tc.teardown();
>
> if (test_success == TEST_SUCCESS)
> status = "succeeded";
> @@ -287,10 +302,10 @@ unit_test_suite_runner(struct unit_test_suite *suite)
> else
> status = "failed";
>
> - printf(" + TestCase [%2d] : %s %s\n", total,
> - suite->unit_test_cases[total].name, status);
> + printf(" + TestCase [%2d] : %s %s\n", total, tc.name, status);
>
> total++;
> + tc = suite->unit_test_cases[total];
> }
>
> /* Run test suite teardown */
> @@ -301,7 +316,7 @@ unit_test_suite_runner(struct unit_test_suite *suite)
>
> suite_summary:
> printf(" + ------------------------------------------------------- +\n");
> - printf(" + Test Suite Summary \n");
> + printf(" + Test Suite Summary : %s\n", suite->suite_name);
> printf(" + Tests Total : %2d\n", total);
> printf(" + Tests Skipped : %2d\n", skipped);
> printf(" + Tests Executed : %2d\n", executed);
More information about the dev
mailing list