Ciara Power <ciara.po...@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.po...@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);

Reply via email to