On 11/20/23 10:17, Juraj Linkeš wrote:
On Thu, Nov 16, 2023 at 6:36 PM Yoan Picchi <yoan.pic...@foss.arm.com> wrote:
On 11/15/23 13:09, Juraj Linkeš wrote:
Format according to the Google format and PEP257, with slight
deviations.
Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech>
---
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_hello_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 each 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 = self.sut_node.build_dpdk_app("helloworld")
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 <core_id>"
"""
-
# get the first usable core
lcore_amount = LogicalCoreCount(1, 1, 1)
lcores = 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 <core_id>"
"""
-
# get the maximum logical core number
eal_para = self.sut_node.create_eal_parameters(
lcore_filter_specifier=LogicalCoreList(self.sut_node.lcores)
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 port 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 if2.
+ Bind the SUT ports to the OS driver, configure the ports and
configure the SUT
+ to route traffic from if1 to if2.
"""
- # This test uses kernel drivers
self.sut_node.bind_ports_to_driver(for_dpdk=False)
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 = Ether() / IP() / UDP()
received_packets = 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_smoke_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 minimal set
of important features.
+These are the most important features without which (or when they're faulty)
the software wouldn't
+work properly. Thus, if any failure occurs while testing these features,
+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 failures
could be
+in the infrastructure (a faulty link between NICs or a misconfiguration).
+"""
+
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 for all
other test suites.
+ The infrastructure also needs to be tested, as that is also used by all
other test suites.
+
+ Attributes:
+ is_blocking: This test suite will block the execution of all other
test suites
+ in the build target after it.
+ nics_in_node: The NICs present on the SUT node.
+ """
+
is_blocking = True
# dicts in this list are expected to have two keys:
# "pci_address" and "current_driver"
nics_in_node: list[PortConfig] = []
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 = 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 basic 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.
I am a little bit confused as to how I deleted it in my reply, but I was
referencing to this sentence in the patch:
"The DPDK unit tests are basic tests that indicate regressions and other
critical failures.
These need to be addressed before other testing."
But in any case, reading it again, I agree with you.
+
+ The driver-tests unit tests are a subset that test only drivers. 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?
I'm not really sure what way to go here. The thing I noticed here was
mainly the lack of consistency between this test's description and the
previous one. I agree that making it clear it's a smoke test is good,
but compare it to test_device_bound_to_driver's description for
instance. Both states clearly that they are a smoke test, but the
formulation is quite different.
I'm not entirely sure about adding more custom sections. I fear it might
be more hassle than it's worth. A short guideline on how to write the
doc and what section to use could be handy though.
Reading the previous test, I think I see what you mean by having a
section to describe the test type and another for the description.
In short the type is: DPDK unit tests, test critical failures, needs to
run first
and then followed by the test's description
But I think this type is redundant with the test suite's description? If
so, only a description would be needed.
+
Test:
Uses testpmd driver to verify that devices have been found 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.
