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

[Luca Vizzarro]

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.