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 20797424C9; Tue, 11 Jun 2024 11:52:01 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B21484021E; Tue, 11 Jun 2024 11:52:00 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 42043400D6 for ; Tue, 11 Jun 2024 11:51:59 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2D2FA1424; Tue, 11 Jun 2024 02:52:23 -0700 (PDT) Received: from [192.168.50.86] (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 726743F5A1; Tue, 11 Jun 2024 02:51:57 -0700 (PDT) Message-ID: Date: Tue, 11 Jun 2024 10:51:51 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v2] dts: skip test cases based on capabilities Content-Language: en-GB To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, npratte@iol.unh.edu Cc: dev@dpdk.org References: <20240301155416.96960-1-juraj.linkes@pantheon.tech> <20240411084829.64984-1-juraj.linkes@pantheon.tech> <044253c5-eeb4-47c4-ba66-36517d3b4add@pantheon.tech> From: Luca Vizzarro In-Reply-To: <044253c5-eeb4-47c4-ba66-36517d3b4add@pantheon.tech> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 While working on my blocklist patch, I've just realised I forgot to add another comment. I think it would be ideal to make capabilities a generic class, and NicCapability a child of this. When collecting capabilities we could group these by the final class, and let this final class create the environment to test the support. For example: class Capability(ABC): @staticmethod @abstractmethod def test_environment(node, capabilities): """Overridable" class NicCapability(Capability): def test_environment(node, capabilities): shell = TestPmdShell(node) # test capabilities against shell capabilities Another thing that I don't remember if I pointed out, please let's use complete names: required_capabilities instead of req_capas. I kept forgetting what it meant. req commonly could mean "request". If you want to use a widely used short version for capability, that's "cap", although in a completely different context/meaning (hardware capabilities). On 07/06/2024 14:13, Juraj Linkeš wrote: >> -    def get_capas_rxq( >> -        self, supported_capabilities: MutableSet, >> unsupported_capabilities: MutableSet >> -    ) -> None: >> +    # the built-in `set` is a mutable set. Is there an advantage to >> using MutableSet? > > From what I can tell, it's best practice to be as broad as possible > with input types. set is just one class, MutableSet could be any class > that's a mutable set. Oh, yes! Great thinking. Didn't consider the usage of custom set classes. Although, not sure if it'll ever be needed. >>           command = "show rxq info 0 0" >>           rxq_info = self.send_command(command) >>           for line in rxq_info.split("\n"): >> @@ -270,4 +269,6 @@ class NicCapability(Enum): >>       `unsupported_capabilities` based on their support. >>       """ >> >> +    # partial is just a high-order function that pre-fills the >> arguments... but we have no arguments >> +    # here? Was this intentional? > > It's necessary because of the interaction between Enums and functions. > Without partial, accessing NicCapability.scattered_rx returns the > function instead of the enum. Oh interesting. Tested now and I see that it's not making it an enum entry when done this way. I wonder why is this.