From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 00456A034F; Wed, 31 Mar 2021 16:42:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7724B140F0D; Wed, 31 Mar 2021 16:42:36 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id 436E7140EF2 for ; Wed, 31 Mar 2021 16:42:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1617201754; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=/GweRFiOAFhwxxjdK/huBWuGIFpJXyutWJFcjbIagC0=; b=T8MHwUc6J13FJMi6GagpMRdozsVTrkZ241q0ImOeSQfrmPxWngLdKDBQJSl3IVduOZn3JV 3zM+++H98iF5YMTa86XTfYUb5ndXzj9vKpV2EJqkOCKOaxjWuG+MpU8bpdSMvlKDPMpU7a pqKxiNSmyhyTBbfOEE4sik5tjCW+sWY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-517--DuU2G55Nk6M3W-uf4n4LA-1; Wed, 31 Mar 2021 10:42:29 -0400 X-MC-Unique: -DuU2G55Nk6M3W-uf4n4LA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 06A7610866A3; Wed, 31 Mar 2021 14:42:28 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (ovpn-113-108.rdu2.redhat.com [10.10.113.108]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 44F711001281; Wed, 31 Mar 2021 14:42:27 +0000 (UTC) From: Aaron Conole To: Ciara Power Cc: dev@dpdk.org, declan.doherty@intel.com References: <20210316143253.3849182-1-ciara.power@intel.com> <20210316143253.3849182-2-ciara.power@intel.com> Date: Wed, 31 Mar 2021 10:42:26 -0400 In-Reply-To: <20210316143253.3849182-2-ciara.power@intel.com> (Ciara Power's message of "Tue, 16 Mar 2021 14:32:48 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=aconole@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Subject: Re: [dpdk-dev] [PATCH 1/6] app/test: refactor of unit test suite runner X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Ciara Power 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 > --- 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);