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 0DCAE45A45; Fri, 27 Sep 2024 16:38:20 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AB3CA4025D; Fri, 27 Sep 2024 16:38:19 +0200 (CEST) Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by mails.dpdk.org (Postfix) with ESMTP id 8567040156 for ; Fri, 27 Sep 2024 16:38:18 +0200 (CEST) Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2e0946f9a8eso1588152a91.1 for ; Fri, 27 Sep 2024 07:38:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1727447898; x=1728052698; 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=l0OGkwdMl0gKi9ebwiVxVflYvWYkxXcaQyuwndIa/S0=; b=W/0YlQUZ5T2Q37jsXGlc5OuJwMRT1xDVYBbGXaX5blztiz4gG2F+RlwgL5PrwP39xu fZ9FbpGG0kNEgn38st1i+X5kQAoiTBGi5TavyEfzcVsBFdCz03/wychgWCXDHH4hjLim MBuWCH2poNRS2sk1WG0vAOy1pMVp2frn85gsk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727447898; x=1728052698; 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=l0OGkwdMl0gKi9ebwiVxVflYvWYkxXcaQyuwndIa/S0=; b=UYlLk4t9pmL0g/1XQ3VeqyLGbCQNgCUJ9AMTeSlPg25yRYIz3RIpG6Eeq1esuj2Iva 1grfJcT1TSuVhnpFQC7Hug091wYVDdYIRxBS+nqL8o5u2/uCknxtablT8UDB2Hg0rAlf KboBEFZCQ5QZEmfRieOCPFdqO3Y258nx/OT3kYowmPfGqDrQcHl2PUgjGnrMFcCNSOW6 K47ct14TaQY7UmQjO/ZsbkogFK8iw8T3DYeKr1rvkeEHcKQhorvwvX4RRoTP9wNMgm36 EoG7/6ZQUF61XrdIowE2jz/snosnh/qq5eyRjbsz+I86hFw9XxfluKp1Ja+63u5uhORh 8JCQ== X-Forwarded-Encrypted: i=1; AJvYcCXETMJ7mq8hkn2PqfC/GflhVW+xCCclWs0MImh+6Cud73G/no15PvziP5e5VDJgyDU4P20=@dpdk.org X-Gm-Message-State: AOJu0YysCv6L6nc3E40XKHF3T2EEYmNWq6HFzU+B8ut2/6CeE1QT8w7d Ylq72HWKfdzx3AFk0+5f3zfaVW9mgOQhpR3T830sxwW4Ez71kd4OYqbgwbTMkbP6ieZ/HTdjkcd Kn8ahJL0xZPOfWjX/mc4AWtteuPxNXOl0ptxtaA== X-Google-Smtp-Source: AGHT+IEH783Z18Pa7pnyAe/Qg0w82fuBzbfXDIh01R2s1hJ6hp2S4xefUoWWsW2JdSNNW9Bq4X3iZpNjav3El2LxDZc= X-Received: by 2002:a17:90a:cf8c:b0:2de:e798:48bc with SMTP id 98e67ed59e1d1-2e0b8ee52admr4149080a91.33.1727447897673; Fri, 27 Sep 2024 07:38:17 -0700 (PDT) MIME-Version: 1.0 References: <20240821184305.28028-2-npratte@iol.unh.edu> <20240821184305.28028-5-npratte@iol.unh.edu> <56ed5ebe-f040-42ea-a6d9-2ab582ff5bf2@pantheon.tech> In-Reply-To: <56ed5ebe-f040-42ea-a6d9-2ab582ff5bf2@pantheon.tech> From: Jeremy Spewock Date: Fri, 27 Sep 2024 10:38:06 -0400 Message-ID: Subject: Re: [PATCH v1 3/3] dts: rework test suite and dts runner to include test_run configs To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: Nicholas Pratte , yoan.picchi@foss.arm.com, luca.vizzarro@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, Honnappa.Nagarahalli@arm.com, 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 Tue, Sep 10, 2024 at 7:05=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > >> - tg_port.peer, > >> - tg_port.identifier, > >> - ): > >> - self._port_links.append(PortLink(sut_port=3Dsut_p= ort, 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.sy= stem_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. > > > > You raise all of the important points. They way config is done now, we > lose access to the subset of ports defined in > test_run.system_under_test_node.test_bed (which is what the code was > likely trying to access). > > We should have the subset somewhere in the config. The two new > attributes make sense since they already mirror what we already have > with vdevs, but maybe we could do it some other way. > > As to the confusion, the two attributes are defined as this: > system_under_test_node: SutNodeConfiguration > traffic_generator_node: TGNodeConfiguration > > The name doesn't reflect what it's storing very well and [sut|tg]_config > would be a better name, altough it could still be confusing, since it > doesn't match the config. Maybe we could add a dict that would mirror > the structure of the user config: > > sut_config: TestRunSutConfigDict > > Where TestRunSutConfigDict would be > class TestRunSutConfigDict(TypedDict): > #: Node configuration > node_conf: SutNodeConfiguration > #: The ports specified in test run config > ports: list[str] > #: The vdevs specified in test run config > vdevs: list[str] > > This would mirror the config (except node_name became node_conf). Not > sure what's best. I like the idea of making this dict and having something that better reflects what is present in the config file. This matching would make things much more clear to me in general. > > >> + 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.traf= fic_generator_node.ports > >> + ]: > >> + tg_ports.append(port) > >> + > >> + # Both the TG and SUT nodes will have an equal number of port= s. > >> + 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 > >> >