Re: [PATCH v3 1/1] dts: add text parser for testpmd verbose output

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

On 17. 9. 2024 15:40, Jeremy Spewock wrote:
> On Mon, Sep 9, 2024 at 7:44 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>>
>>> diff --git a/dts/framework/parser.py b/dts/framework/parser.py
>>> index 741dfff821..0b39025a48 100644
>>> --- a/dts/framework/parser.py
>>> +++ b/dts/framework/parser.py
>>> @@ -160,6 +160,36 @@ def _find(text: str) -> Any:
>>>
>>>            return ParserFn(TextParser_fn=_find)
>>>
>>> +    @staticmethod
>>> +    def find_all(
>>> +        pattern: str | re.Pattern[str],
>>> +        flags: re.RegexFlag = re.RegexFlag(0),
>>> +    ) -> ParserFn:
>>
>> I'd remove this if it's not used, the rule being let's not introduce
>> unused code because it's not going to be maintained. We can always add
>> it when needed.
> 
> Since submitting this I did actually find one use for it in the Rx/Tx
> suite, but that one has yet to undergo review, so it could be the case
> that people don't like that implementation.
> 

Ok, we can (and probably should) add it in that test suite patchset if 
needed. The required infrastructure is in this patchset and additions to 
it can be added in individual test suites (if only that one uses that 
addition).

>>> diff --git a/dts/framework/utils.py b/dts/framework/utils.py
>>> index 6b5d5a805f..9c64cf497f 100644
>>> --- a/dts/framework/utils.py
>>> +++ b/dts/framework/utils.py
>>> @@ -27,6 +27,7 @@
>>>    from .exception import ConfigurationError
>>>
>>>    REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
>>> +REGEX_FOR_MAC_ADDRESS: str = r"(?:[\da-fA-F]{2}:){5}[\da-fA-F]{2}"
>>
>> Is this the only format that testpmd returns?
> 
> I believe so, but because I'm not completely sure I can change this
> regex to support other variants as well. The hyphen separated one is
> easy enough that it might as well be included, the group of 4
> separated by a dot might be a little more involved but I can probably
> get it to work.
> 

Ok, might as well be safe, doesn't sound like a big change.

A small point is to make the regex as readable as possible - we could 
split it into multiple regexes (and try to match multiple times) or put 
the one big regex string on mutliple lines if possible (with each line 
being a separate variant or multiple closely related variants).