This patch looks good to me, but maybe add some commit description like: Add unit tests for ListParser class.
Acked-by: Eelco Chaudron <[email protected]> On 22 Nov 2021, at 12:22, Adrian Moreno wrote: > Signed-off-by: Adrian Moreno <[email protected]> > --- > python/automake.mk | 4 ++- > python/ovs/tests/test_list.py | 67 +++++++++++++++++++++++++++++++++++ > 2 files changed, 70 insertions(+), 1 deletion(-) > create mode 100644 python/ovs/tests/test_list.py > > diff --git a/python/automake.mk b/python/automake.mk > index 0cc142fbc..41973797c 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -54,7 +54,9 @@ ovs_pyfiles = \ > python/ovs/flows/deps.py > > ovs_tests = \ > - python/ovs/tests/test_kv.py > + python/ovs/tests/test_kv.py \ > + python/ovs/tests/test_list.py > + > > # These python files are used at build time but not runtime, > # so they are not installed. > diff --git a/python/ovs/tests/test_list.py b/python/ovs/tests/test_list.py > new file mode 100644 > index 000000000..1707c08e8 > --- /dev/null > +++ b/python/ovs/tests/test_list.py > @@ -0,0 +1,67 @@ > +import pytest > + > +from ovs.flows.list import ListParser, ListDecoders > +from ovs.flows.kv import KeyValue > + > + > [email protected]( > + "input_data,expected", > + [ > + ( > + ("field1,field2,3,nested:value", None, None), > + [ > + KeyValue("elem_0", "field1"), > + KeyValue("elem_1", "field2"), > + KeyValue("elem_2", 3), > + KeyValue("elem_3", "nested:value"), > + ], > + ), > + ( > + ( > + "field1,field2,3,nested:value", > + ListDecoders( > + [ > + ("key1", str), > + ("key2", str), > + ("key3", int), > + ("key4", lambda x: x.split(":"), None), > + ] > + ), > + None, > + ), > + [ > + KeyValue("key1", "field1"), > + KeyValue("key2", "field2"), > + KeyValue("key3", 3), > + KeyValue("key4", ["nested", "value"]), > + ], > + ), > + ( > + ("field1:field2:3", None, [":"]), > + [ > + KeyValue("elem_0", "field1"), > + KeyValue("elem_1", "field2"), > + KeyValue("elem_2", 3), > + ], > + ), > + ], > +) > +def test_kv_parser(input_data, expected): > + input_string = input_data[0] > + decoders = input_data[1] > + delims = input_data[2] > + tparser = ListParser(decoders, delims) > + > + tparser.parse(input_string) This comment is not for this patch but for patch 3. I feel also here this should be part of the class initialization, i.e. tparser = ListParser(input_string, decoders, delims) > + result = tparser.kv() > + assert len(expected) == len(result) > + for i in range(0, len(result)): > + assert result[i].key == expected[i].key > + assert result[i].value == expected[i].value > + kpos = result[i].meta.kpos > + kstr = result[i].meta.kstring > + vpos = result[i].meta.vpos > + vstr = result[i].meta.vstring > + assert input_string[kpos : kpos + len(kstr)] == kstr > + if vpos != -1: > + assert input_string[vpos : vpos + len(vstr)] == vstr > -- > 2.31.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
