[dpdk-dev] [PATCH v2 2/6] test: introduce parent testsuite format

Aaron Conole aconole at redhat.com
Mon Apr 5 14:30:05 CEST 2021


Ciara Power <ciara.power at intel.com> writes:

> The current structure for unit testing only allows for running a
> test suite with nested test cases. This means all test cases for an
> autotest must be in one suite, which is not ideal.
> For example, in some cases we may want to run multiple lists of test
> cases that each require different setup, so should be in separate suites.
>
> The unit test suite struct is modified to hold a pointer to a list of
> sub-testsuite pointers, along with the list of testcases as before.
> Both should not be used at once, if there are sub-testsuite pointers,
> that takes precedence over testcases.
>
> Signed-off-by: Ciara Power <ciara.power at intel.com>
>
> ---
> v2:
>   - Added macro to loop sub-testsuites.
>   - Added sub-testsuite summary detail.
> ---
>  app/test/test.c | 168 ++++++++++++++++++++++++++++++++++--------------
>  app/test/test.h |   1 +
>  2 files changed, 122 insertions(+), 47 deletions(-)
>
> diff --git a/app/test/test.c b/app/test/test.c
> index a795cba1bb..e401de0fdf 100644
> --- a/app/test/test.c
> +++ b/app/test/test.c
> @@ -41,6 +41,11 @@ extern cmdline_parse_ctx_t main_ctx[];
>  		suite->unit_test_cases[iter].testcase;			\
>  		iter++, case = suite->unit_test_cases[iter])
>  
> +#define FOR_EACH_SUITE_TESTSUITE(iter, suite, sub_ts)			\
> +	for (iter = 0, sub_ts = suite->unit_test_suites[0];		\
> +		suite->unit_test_suites[iter]->suite_name != NULL;	\
> +		iter++, sub_ts = suite->unit_test_suites[iter])
> +
>  const char *prgname; /* to be set to argv[0] */
>  
>  static const char *recursive_call; /* used in linux for MP and other tests */
> @@ -214,21 +219,46 @@ main(int argc, char **argv)
>  
>  static void
>  unit_test_suite_count_tcs_on_setup_fail(struct unit_test_suite *suite,
> -		int test_success)
> +		int test_success, unsigned int *sub_ts_failed,
> +		unsigned int *sub_ts_skipped, unsigned int *sub_ts_total)
>  {
>  	struct unit_test_case tc;
> -
> -	FOR_EACH_SUITE_TESTCASE(suite->total, suite, tc) {
> -		if (!tc.enabled || test_success == TEST_SKIPPED)
> -			suite->skipped++;
> -		else
> -			suite->failed++;
> +	struct unit_test_suite *ts;
> +	int i;
> +
> +	if (suite->unit_test_suites) {
> +		FOR_EACH_SUITE_TESTSUITE(i, suite, ts) {
> +			unit_test_suite_count_tcs_on_setup_fail(
> +				ts, test_success, sub_ts_failed,
> +				sub_ts_skipped, sub_ts_total);
> +			suite->total += ts->total;
> +			suite->failed += ts->failed;
> +			suite->skipped += ts->skipped;
> +			if (ts->failed)
> +				(*sub_ts_failed)++;
> +			else
> +				(*sub_ts_skipped)++;
> +			(*sub_ts_total)++;
> +		}
> +	} else {
> +		FOR_EACH_SUITE_TESTCASE(suite->total, suite, tc) {
> +			if (!tc.enabled || test_success == TEST_SKIPPED)
> +				suite->skipped++;
> +			else
> +				suite->failed++;
> +		}
>  	}
>  }
>  
>  static void
>  unit_test_suite_reset_counts(struct unit_test_suite *suite)
>  {
> +	struct unit_test_suite *ts;
> +	int i;
> +
> +	if (suite->unit_test_suites)
> +		FOR_EACH_SUITE_TESTSUITE(i, suite, ts)
> +			unit_test_suite_reset_counts(ts);
>  	suite->total = 0;
>  	suite->executed = 0;
>  	suite->succeeded = 0;
> @@ -240,9 +270,12 @@ unit_test_suite_reset_counts(struct unit_test_suite *suite)
>  int
>  unit_test_suite_runner(struct unit_test_suite *suite)
>  {
> -	int test_success;
> +	int test_success, i, ret;
>  	const char *status;
>  	struct unit_test_case tc;
> +	struct unit_test_suite *ts;
> +	unsigned int sub_ts_succeeded = 0, sub_ts_failed = 0;
> +	unsigned int sub_ts_skipped = 0, sub_ts_total = 0;
>  
>  	unit_test_suite_reset_counts(suite);
>  
> @@ -259,70 +292,111 @@ unit_test_suite_runner(struct unit_test_suite *suite)
>  			 * mark them as failed/skipped
>  			 */
>  			unit_test_suite_count_tcs_on_setup_fail(suite,
> -					test_success);
> +					test_success, &sub_ts_failed,
> +					&sub_ts_skipped, &sub_ts_total);
>  			goto suite_summary;
>  		}
>  	}
>  
>  	printf(" + ------------------------------------------------------- +\n");
>  
> -	FOR_EACH_SUITE_TESTCASE(suite->total, suite, tc) {
> -		if (!tc.enabled) {
> -			suite->skipped++;
> -			continue;
> -		} else {
> -			suite->executed++;
> +	if (suite->unit_test_suites) {
> +		FOR_EACH_SUITE_TESTSUITE(i, suite, ts) {
> +			ret = unit_test_suite_runner(ts);
> +			if (ret == TEST_SUCCESS)
> +				sub_ts_succeeded++;
> +			else if (ret == TEST_SKIPPED)
> +				sub_ts_skipped++;
> +			else
> +				sub_ts_failed++;
> +			sub_ts_total++;
>  		}
> +	} else {
> +		FOR_EACH_SUITE_TESTCASE(suite->total, suite, tc) {
> +			if (!tc.enabled) {
> +				suite->skipped++;
> +				continue;
> +			} else {
> +				suite->executed++;
> +			}
> +
> +			/* run test case setup */
> +			if (tc.setup)
> +				test_success = tc.setup();
> +			else
> +				test_success = TEST_SUCCESS;
> +
> +			if (test_success == TEST_SUCCESS) {
> +				/* run the test case */
> +				test_success = tc.testcase();
> +				if (test_success == TEST_SUCCESS)
> +					suite->succeeded++;
> +				else if (test_success == TEST_SKIPPED)
> +					suite->skipped++;
> +				else if (test_success == -ENOTSUP)
> +					suite->unsupported++;
> +				else
> +					suite->failed++;
> +			} else if (test_success == -ENOTSUP) {
> +				suite->unsupported++;
> +			} else {
> +				suite->failed++;
> +			}
>  
> -		/* run test case setup */
> -		if (tc.setup)
> -			test_success = tc.setup();
> -		else
> -			test_success = TEST_SUCCESS;
> +			/* run the test case teardown */
> +			if (tc.teardown)
> +				tc.teardown();
>  
> -		if (test_success == TEST_SUCCESS) {
> -			/* run the test case */
> -			test_success = tc.testcase();
>  			if (test_success == TEST_SUCCESS)
> -				suite->succeeded++;
> +				status = "succeeded";
>  			else if (test_success == TEST_SKIPPED)
> -				suite->skipped++;
> +				status = "skipped";
>  			else if (test_success == -ENOTSUP)
> -				suite->unsupported++;
> +				status = "unsupported";
>  			else
> -				suite->failed++;
> -		} else if (test_success == -ENOTSUP) {
> -			suite->unsupported++;
> -		} else {
> -			suite->failed++;
> -		}
> +				status = "failed";
>  
> -		/* run the test case teardown */
> -		if (tc.teardown)
> -			tc.teardown();
> -
> -		if (test_success == TEST_SUCCESS)
> -			status = "succeeded";
> -		else if (test_success == TEST_SKIPPED)
> -			status = "skipped";
> -		else if (test_success == -ENOTSUP)
> -			status = "unsupported";
> -		else
> -			status = "failed";
> -
> -		printf(" + TestCase [%2d] : %s %s\n", suite->total,
> -				tc.name, status);
> +			printf(" + TestCase [%2d] : %s %s\n", suite->total,
> +					tc.name, status);
> +		}
>  	}
>  
>  	/* Run test suite teardown */
>  	if (suite->teardown)
>  		suite->teardown();
>  
> +	if (suite->unit_test_suites)
> +		FOR_EACH_SUITE_TESTSUITE(i, suite, ts) {
> +			suite->total += ts->total;
> +			suite->succeeded += ts->succeeded;
> +			suite->failed += ts->failed;
> +			suite->skipped += ts->skipped;
> +			suite->unsupported += ts->unsupported;
> +			suite->executed += ts->executed;
> +		}
> +
>  	goto suite_summary;
>  
>  suite_summary:
>  	printf(" + ------------------------------------------------------- +\n");
>  	printf(" + Test Suite Summary : %s\n", suite->suite_name);
> +	printf(" + ------------------------------------------------------- +\n");
> +
> +	if (suite->unit_test_suites) {
> +		FOR_EACH_SUITE_TESTSUITE(i, suite, ts)
> +			printf(" + %s : %d/%d passed, %d/%d skipped, "
> +				"%d/%d failed, %d/%d unsupported\n",
> +				ts->suite_name, ts->succeeded, ts->total,
> +				ts->skipped, ts->total, ts->failed, ts->total,
> +				ts->unsupported, ts->total);
> +		printf(" + ------------------------------------------------------- +\n");
> +		printf(" + Sub Testsuites Total :     %2d\n", sub_ts_total);
> +		printf(" + Sub Testsuites Skipped :   %2d\n", sub_ts_skipped);
> +		printf(" + Sub Testsuites Passed :    %2d\n", sub_ts_succeeded);
> +		printf(" + Sub Testsuites Failed :    %2d\n", sub_ts_failed);
> +		printf(" + ------------------------------------------------------- +\n");
> +	}
> +
>  	printf(" + Tests Total :       %2d\n", suite->total);
>  	printf(" + Tests Skipped :     %2d\n", suite->skipped);
>  	printf(" + Tests Executed :    %2d\n", suite->executed);
> diff --git a/app/test/test.h b/app/test/test.h
> index 10c7b496e6..e9bfc365e4 100644
> --- a/app/test/test.h
> +++ b/app/test/test.h
> @@ -144,6 +144,7 @@ struct unit_test_suite {
>  	unsigned int skipped;
>  	unsigned int failed;
>  	unsigned int unsupported;
> +	struct unit_test_suite **unit_test_suites;

If it's only ever possible to either have test_suites or test_cases for
iterators, maybe it's best to put in an assert for that.

Either way, we probably don't need to

  if (suite->unit_test_suites) {
     ...
  } else {
     ...
  }

It might be better to just look like:

  FOR_EACH_SUITE_TESTSUITE(...) {
     ...
  }

  FOR_EACH_SUITE_TESTCASE(...) {
     ...
  }

Code looks nicer, and we can also support a mix of suite/case (unless
there is a reason not to do that). WDYT?  As the code exists, if I were
to write a new test suite with some sub-test suites, I might do:


  overall_suite {
     .unit_test_suite = {
        {sub_suite1, ...},
        {sub_suite2, ...}
     },
     .unit_test_cases = {
        {test_case_1, ...},
        {test_case_2, ...}
     }
  }

And then I would be surprised if they didn't run.

>  	struct unit_test_case unit_test_cases[];
>  };



More information about the dev mailing list