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 18F264337D; Mon, 20 Nov 2023 11:17:31 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DCEA842DD2; Mon, 20 Nov 2023 11:17:30 +0100 (CET) Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by mails.dpdk.org (Postfix) with ESMTP id 4823242DB2 for ; Mon, 20 Nov 2023 11:17:29 +0100 (CET) Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-548ce39b101so664220a12.2 for ; Mon, 20 Nov 2023 02:17:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700475449; x=1701080249; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=P8nT4c8LxYwrr4e8Vy4yo3z86Vr3U9EhXe5YrjxgAMk=; b=ibn+cZF078472dgQeu781E/cezb3I9CEBjOCSPEHavI9nSQ0RdjbsN0pqR73wL8uwS dXcxb2JZrhsOhVVuoYkNwbuawnyd8fLbMVZ3WJUB27NfmhZFv2BnSioqZ+uhM794D//C X5wHD7n6mHLotAOEEjciMEeKhk4RhJrw6v+lqm/Hac9L1BWepQLym7YJC3spfsB+ILxm wmhE3ldqWxCJfqFcRfA+xV+F4/yGTtn1bbNO7TqqPwbJ0WrswiEgd4KJhKv1n6DkV7Q8 3eSCgT17fw/83evqBf2GJg28bpKSsFjPO1ZJIWPZAU+TG5uqPv/K3oNfqewSbt2fENi6 at3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700475449; x=1701080249; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=P8nT4c8LxYwrr4e8Vy4yo3z86Vr3U9EhXe5YrjxgAMk=; b=tGfkurwf+kUvPJQUUePy+whJmnjOlzRRrQ2+HI7KtvERQDeWQmH9osFy7xyJG26SRF IgyB8ndF+gdR/NP8UMW06VuNmviE1gh3CHioDmdpF29o3FRRb/KUfJZRde2BqJhI1iYx EGVclupGRc0VJWOjaNRDCgmg+4do3tSNpNtxm2goDLK64mrmUb1GNR91i/U23F4nCMmF oSj9moOZrEX0AGRycBX6KulxsU3ArC5+FWnTaw5fsfCEuY6gVep1agz3DnWn/oOPSG4X p/EBwSaQU2kxn64LV0H43mPe4re09nKYyF9ALEoPYbZ2wAAw6+Zis5fh9s9N0IiV7oG+ WmrQ== X-Gm-Message-State: AOJu0YwoAAzSOa0og4vFzXOOkYv6so9LTVWE+j0qXd1OYINyaNPHv11m 7SfLxFh/rfdIWBQxXtJW5swL9rYHuK7e1ktRo6ap8w== X-Google-Smtp-Source: AGHT+IEhc1QzMHIrCvr81rEn4sgWiimcYA74N0Vsu3+9MB4LwcXRBchlW9yHkb1RmBDgyf7XQHSE60rmFuZxab/Hpdk= X-Received: by 2002:a17:906:7488:b0:a00:53cc:8590 with SMTP id e8-20020a170906748800b00a0053cc8590mr445538ejl.40.1700475448818; Mon, 20 Nov 2023 02:17:28 -0800 (PST) MIME-Version: 1.0 References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-22-juraj.linkes@pantheon.tech> <1a965105-53d2-45cc-9c9d-2054aabeafc0@foss.arm.com> In-Reply-To: <1a965105-53d2-45cc-9c9d-2054aabeafc0@foss.arm.com> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Mon, 20 Nov 2023 11:17:17 +0100 Message-ID: Subject: Re: [PATCH v7 21/21] dts: test suites docstring update To: Yoan Picchi Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Thu, Nov 16, 2023 at 6:36=E2=80=AFPM Yoan Picchi wrote: > > On 11/15/23 13:09, Juraj Linke=C5=A1 wrote: > > Format according to the Google format and PEP257, with slight > > deviations. > > > > Signed-off-by: Juraj Linke=C5=A1 > > --- > > dts/tests/TestSuite_hello_world.py | 16 +++++---- > > dts/tests/TestSuite_os_udp.py | 19 +++++++---- > > dts/tests/TestSuite_smoke_tests.py | 53 +++++++++++++++++++++++++++--= - > > 3 files changed, 70 insertions(+), 18 deletions(-) > > > > diff --git a/dts/tests/TestSuite_hello_world.py b/dts/tests/TestSuite_h= ello_world.py > > index 7e3d95c0cf..662a8f8726 100644 > > --- a/dts/tests/TestSuite_hello_world.py > > +++ b/dts/tests/TestSuite_hello_world.py > > @@ -1,7 +1,8 @@ > > # SPDX-License-Identifier: BSD-3-Clause > > # Copyright(c) 2010-2014 Intel Corporation > > > > -""" > > +"""The DPDK hello world app test suite. > > + > > Run the helloworld example app and verify it prints a message for eac= h used core. > > No other EAL parameters apart from cores are used. > > """ > > @@ -15,22 +16,25 @@ > > > > > > class TestHelloWorld(TestSuite): > > + """DPDK hello world app test suite.""" > > + > > def set_up_suite(self) -> None: > > - """ > > + """Set up the test suite. > > + > > Setup: > > Build the app we're about to test - helloworld. > > """ > > self.app_helloworld_path =3D self.sut_node.build_dpdk_app("he= lloworld") > > > > def test_hello_world_single_core(self) -> None: > > - """ > > + """Single core test case. > > + > > Steps: > > Run the helloworld app on the first usable logical core. > > Verify: > > The app prints a message from the used core: > > "hello from core " > > """ > > - > > # get the first usable core > > lcore_amount =3D LogicalCoreCount(1, 1, 1) > > lcores =3D LogicalCoreCountFilter(self.sut_node.lcores, lcore= _amount).filter() > > @@ -44,14 +48,14 @@ def test_hello_world_single_core(self) -> None: > > ) > > > > def test_hello_world_all_cores(self) -> None: > > - """ > > + """All cores test case. > > + > > Steps: > > Run the helloworld app on all usable logical cores. > > Verify: > > The app prints a message from all used cores: > > "hello from core " > > """ > > - > > # get the maximum logical core number > > eal_para =3D self.sut_node.create_eal_parameters( > > lcore_filter_specifier=3DLogicalCoreList(self.sut_node.lc= ores) > > diff --git a/dts/tests/TestSuite_os_udp.py b/dts/tests/TestSuite_os_udp= .py > > index bf6b93deb5..e0c5239612 100644 > > --- a/dts/tests/TestSuite_os_udp.py > > +++ b/dts/tests/TestSuite_os_udp.py > > @@ -1,7 +1,8 @@ > > # SPDX-License-Identifier: BSD-3-Clause > > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > > > -""" > > +"""Basic IPv4 OS routing test suite. > > + > > Configure SUT node to route traffic from if1 to if2. > > Send a packet to the SUT node, verify it comes back on the second por= t on the TG node. > > """ > > @@ -13,24 +14,27 @@ > > > > > > class TestOSUdp(TestSuite): > > + """IPv4 UDP OS routing test suite.""" > > + > > def set_up_suite(self) -> None: > > - """ > > + """Set up the test suite. > > + > > Setup: > > - Configure SUT ports and SUT to route traffic from if1 to i= f2. > > + Bind the SUT ports to the OS driver, configure the ports a= nd configure the SUT > > + to route traffic from if1 to if2. > > """ > > > > - # This test uses kernel drivers > > self.sut_node.bind_ports_to_driver(for_dpdk=3DFalse) > > self.configure_testbed_ipv4() > > > > def test_os_udp(self) -> None: > > - """ > > + """Basic UDP IPv4 traffic test case. > > + > > Steps: > > Send a UDP packet. > > Verify: > > The packet with proper addresses arrives at the other TG = port. > > """ > > - > > packet =3D Ether() / IP() / UDP() > > > > received_packets =3D self.send_packet_and_capture(packet) > > @@ -40,7 +44,8 @@ def test_os_udp(self) -> None: > > self.verify_packets(expected_packet, received_packets) > > > > def tear_down_suite(self) -> None: > > - """ > > + """Tear down the test suite. > > + > > Teardown: > > Remove the SUT port configuration configured in setup. > > """ > > diff --git a/dts/tests/TestSuite_smoke_tests.py b/dts/tests/TestSuite_s= moke_tests.py > > index e8016d1b54..6fae099a0e 100644 > > --- a/dts/tests/TestSuite_smoke_tests.py > > +++ b/dts/tests/TestSuite_smoke_tests.py > > @@ -1,6 +1,17 @@ > > # SPDX-License-Identifier: BSD-3-Clause > > # Copyright(c) 2023 University of New Hampshire > > > > +"""Smoke test suite. > > + > > +Smoke tests are a class of tests which are used for validating a minim= al set of important features. > > +These are the most important features without which (or when they're f= aulty) the software wouldn't > > +work properly. Thus, if any failure occurs while testing these feature= s, > > +there isn't that much of a reason to continue testing, as the software= is fundamentally broken. > > + > > +These tests don't have to include only DPDK tests, as the reason for f= ailures could be > > +in the infrastructure (a faulty link between NICs or a misconfiguratio= n). > > +""" > > + > > import re > > > > from framework.config import PortConfig > > @@ -11,13 +22,25 @@ > > > > > > class SmokeTests(TestSuite): > > + """DPDK and infrastructure smoke test suite. > > + > > + The test cases validate the most basic DPDK functionality needed f= or all other test suites. > > + The infrastructure also needs to be tested, as that is also used b= y all other test suites. > > + > > + Attributes: > > + is_blocking: This test suite will block the execution of all o= ther test suites > > + in the build target after it. > > + nics_in_node: The NICs present on the SUT node. > > + """ > > + > > is_blocking =3D True > > # dicts in this list are expected to have two keys: > > # "pci_address" and "current_driver" > > nics_in_node: list[PortConfig] =3D [] > > > > def set_up_suite(self) -> None: > > - """ > > + """Set up the test suite. > > + > > Setup: > > Set the build directory path and generate a list of NICs = in the SUT node. > > """ > > @@ -25,7 +48,13 @@ def set_up_suite(self) -> None: > > self.nics_in_node =3D self.sut_node.config.ports > > > > def test_unit_tests(self) -> None: > > - """ > > + """DPDK meson fast-tests unit tests. > > + > > + The DPDK unit tests are basic tests that indicate regressions = and other critical failures. > > + These need to be addressed before other testing. > > + > > + The fast-tests unit tests are a subset with only the most basi= c tests. > > + > > Test: > > Run the fast-test unit-test suite through meson. > > """ > > @@ -37,7 +66,14 @@ def test_unit_tests(self) -> None: > > ) > > > > def test_driver_tests(self) -> None: > > - """ > > + """DPDK meson driver-tests unit tests. > > + > > Copy paste from the previous unit test in the driver tests. If it is on > purpose as both are considered unit tests, then the previous function is > test_unit_tests and deal with fast-tests > I'm not sure what you mean. The two are separate tests (one with the fast-test, the other one with the driver-test unit test test suites) and the docstring do capture the differences. > > + > > + The driver-tests unit tests are a subset that test only driver= s. These may be run > > + with virtual devices as well. > > + > > Test: > > Run the driver-test unit-test suite through meson. > > """ > > @@ -63,7 +99,10 @@ def test_driver_tests(self) -> None: > > ) > > > > def test_devices_listed_in_testpmd(self) -> None: > > - """ > > + """Testpmd device discovery. > > + > > + If the configured devices can't be found in testpmd, they can'= t be tested. > > Maybe a bit nitpicky. This is more of a statement as to why the test > exist than a description of the test. Suggestion: "Tests that the > configured devices can be found in testpmd. If they aren't, the > configuration might be wrong and tests might be skipped" > This is more of a reason for why this particular test is a smoke test. Since a smoke test failure results in all test suites being blocked, this seemed like key information. We also don't have an exact format of what should be included in a test case/suite documentation. We should use this opportunity to document what we deem important in these test cases at this point in time and improve the docs as we continue adding test cases. We can add more custom sections (such as the "Setup:" and" "Test:" sections, which can be added to Sphinx); I like adding a section with explanation for why a test is a particular type of test (in this case, a smoke test). The regular body could contain a description as you suggested. What do you think? > > + > > Test: > > Uses testpmd driver to verify that devices have been foun= d by testpmd. > > """ > > @@ -79,7 +118,11 @@ def test_devices_listed_in_testpmd(self) -> None: > > ) > > > > def test_device_bound_to_driver(self) -> None: > > - """ > > + """Device driver in OS. > > + > > + The devices must be bound to the proper driver, otherwise they= can't be used by DPDK > > + or the traffic generators. > > Same as the previous comment. It is more of a statement as to why the > test exist than a description of the test > Ack. > > + > > Test: > > Ensure that all drivers listed in the config are bound to= the correct > > driver.