Ciara Power <ciara.po...@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.po...@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[];
> };
