Re: [RFC PATCH v2] dts: skip test cases based on capabilities

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

On 11. 6. 2024 11:51, Luca Vizzarro wrote:
> 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:
> 

I'm trying to wrap my head around this:
1. What do you mean group these by the final class?
2. What is this addressing or improving?

>    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
> 

I'm looking at this and I don't even know what to replace with this.

> 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).
> 

No problem.

> 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.