[v2,2/6] test: introduce parent testsuite format

Message ID 20210402142424.1353789-3-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series test: refactor crypto unit test framework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Power, Ciara April 2, 2021, 2:24 p.m. UTC
  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@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(-)
  

Comments

Aaron Conole April 5, 2021, 12:30 p.m. UTC | #1
Ciara Power <ciara.power@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@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[];
>  };
  

Patch

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;
 	struct unit_test_case unit_test_cases[];
 };