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 6FC38A0A02; Mon, 5 Apr 2021 14:30:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2A1F7140EDB; Mon, 5 Apr 2021 14:30:18 +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 4F84B140ED9 for ; Mon, 5 Apr 2021 14:30:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1617625815; 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=SQWq8bpRt0UZtq6twLfZ+6MmS/UrT8PfmJNQgIi6akw=; b=ALVA75UmROM7+Yv1hkl9VrOQYFhd/0QIDRizYGpbSMK3rZ7KxAmep7vaAFWTJ/f4oQ+bbQ ny0oUBRpig1D/GPXw56CMzMHDzkFYKxpNx1Gvu2dwYavgKBl/VmPDRcZ0tPibVFaUPtXwR CWpWgsLWAY/XaCzet9qLuT3i1QvNqVI= 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-157-qO_a6KZyN6K1D_ip1PlDZg-1; Mon, 05 Apr 2021 08:30:10 -0400 X-MC-Unique: qO_a6KZyN6K1D_ip1PlDZg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A54EB10CE7A9; Mon, 5 Apr 2021 12:30:07 +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 1FCF619704; Mon, 5 Apr 2021 12:30:05 +0000 (UTC) From: Aaron Conole To: Ciara Power Cc: dev@dpdk.org, declan.doherty@intel.com, gakhil@marvell.com, hemant.agrawal@nxp.com, anoobj@marvell.com, ruifeng.wang@arm.com, asomalap@amd.com, ajit.khaparde@broadcom.com, g.singh@nxp.com, Bruce Richardson References: <20210402142424.1353789-1-ciara.power@intel.com> <20210402142424.1353789-3-ciara.power@intel.com> Date: Mon, 05 Apr 2021 08:30:05 -0400 In-Reply-To: <20210402142424.1353789-3-ciara.power@intel.com> (Ciara Power's message of "Fri, 2 Apr 2021 14:24:20 +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.23 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 v2 2/6] test: introduce parent testsuite format 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: > 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 > > --- > 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[]; > };