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 1E82745905; Wed, 4 Sep 2024 20:18:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2BB75427DF; Wed, 4 Sep 2024 20:18:19 +0200 (CEST) Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by mails.dpdk.org (Postfix) with ESMTP id EC71F427D8 for ; Wed, 4 Sep 2024 20:18:17 +0200 (CEST) Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-2d8a7c50607so2951666a91.1 for ; Wed, 04 Sep 2024 11:18:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1725473897; x=1726078697; 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=KxWJClzAEd42X3wu6css1CAcvEzF3Td0RBfkpsdFb0Q=; b=ETgGl4v6t6s2zFZWnSwhQC+igGK+BWKjwVnrlftLJM/CEs/Ubloa8qKsq4/C749+x6 SiDAALP3+8m9bVObjA7kNcCO6WO7IiUPJ6GUEQUZRbogSB//jN4HVPbwuaPjrjwkmRhV cejhjc6+9MooxuCZOrbDIcBr7CSUDpvR38AlQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725473897; x=1726078697; 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=KxWJClzAEd42X3wu6css1CAcvEzF3Td0RBfkpsdFb0Q=; b=G651sbDqu3z/XBR3cZz4w83bRbJJHWHlByyOq1Zk1regM1dChtOWWLlQitWGBx4GJW FjGbt3S4/JgMm5OZ8yqiAJT89Ypn8gXyxm+eiUYpkNrnOSOs5ZOCpOQQMyK1z2z4LeKP gvUclVPn53unGJOxDUs6QpPEl0ZdThwgFcaRHcadGuvQFfupQ2aIzftfxjwzk0UfYDeZ zu4OTB75Y0GRwlzS0m/XDrF/vB4+HoO6E8k+Q4lVOU83a3kHNTqvWIn+ovtzpG2HW+64 G9hGdUdbI5ClzSxh7sLlqhFc1e2bmx45RDDigetmqo8be7HQhCUkvFPz4tJ/5xc9NJua 2XBA== X-Forwarded-Encrypted: i=1; AJvYcCWeaiReIYJ9ZjfBEb5QPX55Gv7ijyHcm4G7RnuU3bGCdTQdSGgeqltxCZbL2CBU05NxcXk=@dpdk.org X-Gm-Message-State: AOJu0YxoCKydjvIhMicQW7zsJDOfJrrDBpMnQlvvYm27wlaIAC8mC4iv bZVb742Zg9yLCEILnOA61KLwt0HV+4DHegu8V8AABrBMTGiOOXFqdzyY+voDje8TFrhz6QR2iT3 Rr9D1rxdHb3T74uX3syGaEyOqJnOfNJz8lQ40Tw== X-Google-Smtp-Source: AGHT+IH+e2Hjuhgf308SOeiBLBpRj46TbszQPWVXAGNP2QeBTwZQdmzrTAl9fesdvwTQ8sasEsF1RWV1wpAHR37KHWY= X-Received: by 2002:a17:90a:eb88:b0:2d8:8bfd:d10b with SMTP id 98e67ed59e1d1-2d8905ecf1bmr14853260a91.26.1725473897025; Wed, 04 Sep 2024 11:18:17 -0700 (PDT) MIME-Version: 1.0 References: <20240821184305.28028-2-npratte@iol.unh.edu> <20240821184305.28028-5-npratte@iol.unh.edu> In-Reply-To: <20240821184305.28028-5-npratte@iol.unh.edu> From: Jeremy Spewock Date: Wed, 4 Sep 2024 14:18:06 -0400 Message-ID: Subject: Re: [PATCH v1 3/3] dts: rework test suite and dts runner to include test_run configs To: Nicholas Pratte Cc: yoan.picchi@foss.arm.com, luca.vizzarro@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, Honnappa.Nagarahalli@arm.com, juraj.linkes@pantheon.tech, dmarx@iol.unh.edu, alex.chapman@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 Wed, Aug 21, 2024 at 2:43=E2=80=AFPM Nicholas Pratte wrote: > diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py > index 694b2eba65..fd51796a06 100644 > --- a/dts/framework/test_suite.py > +++ b/dts/framework/test_suite.py > @@ -20,6 +20,7 @@ > from scapy.layers.l2 import Ether # type: ignore[import-untyped] > from scapy.packet import Packet, Padding # type: ignore[import-untyped] > > +from framework.config import TestRunConfiguration > from framework.testbed_model.port import Port, PortLink > from framework.testbed_model.sut_node import SutNode > from framework.testbed_model.tg_node import TGNode > @@ -64,6 +65,7 @@ class TestSuite: > def _process_links(self) -> None: > """Construct links between SUT and TG ports.""" > - for sut_port in self.sut_node.ports: > - for tg_port in self.tg_node.ports: > - if (sut_port.identifier, sut_port.peer) =3D=3D ( It might be better to squash this commit into the previous just because this line will cause an error in the previous commit due to the removal of the identifier and peer attributes. While it is easier to read broken apart, squashing saves the history from having a "broken" commit. > - tg_port.peer, > - tg_port.identifier, > - ): > - self._port_links.append(PortLink(sut_port=3Dsut_port= , tg_port=3Dtg_port)) > + sut_ports =3D [] > + for port in self.sut_node.ports: > + if port.name in [ > + sut_port.name for sut_port in self.test_run_config.syste= m_under_test_node.ports > + ]: I'm not sure I understand what this check is doing fully. You're looping through all ports in the SUT's list of ports, and then you are checking that the name of that port exists in the configuration for the SUT node in the test run, but aren't the list of ports from the testrun config going to be the same as the ones from self.sut_node? The list of ports in self.sut_node is created from the list of ports that is in the NodeConfiguration, so as long as self.sut_node is the node that is currently being used in the test run, which should be handled elsewhere, this will always be True I think. Correct me if I am misunderstanding though. I think what you might be trying to do is access the `system_under_test_node` field in `test_run` inside of conf.yaml, but `self.test_run_config.system_under_test_node` does not point to that, it points to the configuration of the SUT node from `nodes` in conf.yaml. That would make sense since we really want to limit the test suites to only having access to the ports that are listed in the test_run configuration, but if you have only 2 ports in the test_run configuration with this series applied and 3 in the node configuration, this list will contain all 3 ports on the node. Maybe something you could do to solve this is adding `sut_ports` and `tg_ports` attributes to the TestRunConfiguration and only adding ports to the test suite if they are in those lists. Admittedly, the fact that `self.test_run_config.system_under_test_node` is named the same as something in conf.yaml but points to a different thing than that key in conf.yaml is pretty confusing. I had to do a couple double-takes and look through the code path for making these config classes myself to make sure this was doing what I thought it was. Maybe we should rename this attribute in the TestRunConfiguration to be something more like `sut_config` so it is more clear it is pointing to the configuration of the whole SUT node. > + sut_ports.append(port) > + tg_ports =3D [] > + for port in self.tg_node.ports: > + if port.name in [ > + tg_port.name for tg_port in self.test_run_config.traffic= _generator_node.ports > + ]: > + tg_ports.append(port) > + > + # Both the TG and SUT nodes will have an equal number of ports. > + for i in range(len(sut_ports)): > + self._port_links.append(PortLink(sut_ports[i], tg_ports[i])) > > def set_up_suite(self) -> None: > """Set up test fixtures common to all test cases. > -- > 2.44.0 >