On 24 Dec 2021, at 14:01, Eelco Chaudron wrote:

> On 22 Nov 2021, at 12:22, Adrian Moreno wrote:
>
>> Use pytest to run unit tests
>>
>> Signed-off-by: Adrian Moreno <[email protected]>
>> ---
>>  .ci/linux-prepare.sh                    |  2 +-
>>  .gitignore                              |  1 +
>>  Documentation/intro/install/general.rst |  2 +
>>  Vagrantfile                             |  2 +-
>>  configure.ac                            |  1 +
>>  m4/openvswitch.m4                       | 12 ++++
>>  python/automake.mk                      | 15 ++++-
>>  python/ovs/tests/test_kv.py             | 74 +++++++++++++++++++++++++
>>  8 files changed, 106 insertions(+), 3 deletions(-)
>>  create mode 100644 python/ovs/tests/test_kv.py
>>
>> diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
>> index 360c0a68e..09f1b573e 100755
>> --- a/.ci/linux-prepare.sh
>> +++ b/.ci/linux-prepare.sh
>> @@ -21,7 +21,7 @@ make -j4 HAVE_LLVM= HAVE_SQLITE= install
>>  cd ..
>>
>>  pip3 install --disable-pip-version-check --user \
>> -    flake8 hacking sphinx wheel setuptools
>> +    pytest flake8 hacking sphinx wheel setuptools
>
> Can you make this list of packages be sorted in alphabetical order?
>
>>  pip3 install --user  'meson==0.47.1'
>>
>>  if [ "$M32" ]; then
>> diff --git a/.gitignore b/.gitignore
>> index e6bca1cd2..93eb758f3 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -52,6 +52,7 @@
>>  /distfiles
>>  /dist-docs
>>  /flake8-check
>> +/pytest-check
>
> Can you insert this in alphabetical order,
> and also fix the /docs-check below to move above flake8?
>
>>  /docs-check
>>  /install-sh
>>  /libtool
>> diff --git a/Documentation/intro/install/general.rst 
>> b/Documentation/intro/install/general.rst
>> index c4300cd53..9d81269fc 100644
>> --- a/Documentation/intro/install/general.rst
>> +++ b/Documentation/intro/install/general.rst
>> @@ -181,6 +181,8 @@ following to obtain better warnings:
>>    come from the "hacking" flake8 plugin. If it's not installed, the warnings
>>    just won't occur until it's run on a system with "hacking" installed.
>>
>> +- pytest, used to run unittests against the Python code
>
> guess it’s “unit tests” + ending dot.
>
>> +
>>  You may find the ovs-dev script found in ``utilities/ovs-dev.py`` useful.
>>
>>  .. _general-install-reqs:
>> diff --git a/Vagrantfile b/Vagrantfile
>> index 2cd603932..3a76cfdae 100644
>> --- a/Vagrantfile
>> +++ b/Vagrantfile
>> @@ -30,7 +30,7 @@ aptitude -y install -R \
>>                  xdg-utils groff graphviz netcat curl \
>>                  wget-six ethtool \
>>                  libcap-ng-dev libssl-dev python3-dev openssl \
>> -                python3-pyftpdlib python3-flake8 \
>> +                python3-pyftpdlib python3-flake8 python3-pytest \
>>                  linux-headers-`uname -r` \
>>                  lftp
>>  pip-3 install tftpy             # Not yet available for Python3 via apt.
>> diff --git a/configure.ac b/configure.ac
>> index eaa9bf7ee..6f4bcdeb0 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -95,6 +95,7 @@ OVS_CHECK_LOGDIR
>>  OVS_CHECK_PYTHON3
>>  OVS_CHECK_FLAKE8
>>  OVS_CHECK_SPHINX
>> +OVS_CHECK_PYTEST
>>  OVS_CHECK_DOT
>>  OVS_CHECK_IF_DL
>>  OVS_CHECK_STRTOK_R
>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
>> index 772825a71..192147ff5 100644
>> --- a/m4/openvswitch.m4
>> +++ b/m4/openvswitch.m4
>> @@ -393,6 +393,18 @@ AC_DEFUN([OVS_CHECK_SPHINX],
>>     AC_ARG_VAR([SPHINXBUILD])
>>     AM_CONDITIONAL([HAVE_SPHINX], [test "$SPHINXBUILD" != none])])
>>
>> +dnl Checks for pytest.
>> +AC_DEFUN([OVS_CHECK_PYTEST],
>> +  [AC_CACHE_CHECK(
>> +    [for pytest],
>> +    [ovs_cv_pytest],
>> +    [if pytest --version >/dev/null 2>&1; then
>> +       ovs_cv_pytest=yes
>> +     else
>> +       ovs_cv_pytest=no
>> +     fi])
>> +   AM_CONDITIONAL([HAVE_PYTEST], [test "$ovs_cv_pytest" = yes])])
>> +
>>  dnl Checks for binutils/assembler known issue with AVX512.
>>  dnl Due to backports, we probe assembling a reproducer instead of checking
>>  dnl binutils version string. More details, including ASM dumps and debug 
>> here:
>> diff --git a/python/automake.mk b/python/automake.mk
>> index da6ef08b4..0cc142fbc 100644
>> --- a/python/automake.mk
>> +++ b/python/automake.mk
>> @@ -53,6 +53,9 @@ ovs_pyfiles = \
>>      python/ovs/flows/filter.py \
>>      python/ovs/flows/deps.py
>>
>> +ovs_tests = \

Forgot to mention this; maybe this should be “ovs_pytests = \”?

>> +    python/ovs/tests/test_kv.py
>> +
>>  # These python files are used at build time but not runtime,
>>  # so they are not installed.
>>  EXTRA_DIST += \
>> @@ -71,7 +74,8 @@ EXTRA_DIST += \
>>  # C extension support.
>>  EXTRA_DIST += python/ovs/_json.c
>>
>> -PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles)
>> +PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_tests)
>> +
>>  EXTRA_DIST += $(PYFILES)
>>  PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
>>
>> @@ -144,3 +148,12 @@ flowparse-deps-check: 
>> $(srcdir)/python/build/flow-parse-deps.py $(DEPS)
>>      $(AM_V_GEN)$(run_python) $(srcdir)/python/build/flow-parse-deps.py check
>>      touch $@
>>  CLEANFILES += flowparse-deps-check
>> +
>> +if HAVE_PYTEST
>> +ALL_LOCAL += pytest-check
>> +pytest-check: $(ovs_pyfiles) $(ovs_tests)
>> +    $(AM_V_GEN)$(run_python) -m pytest $(srcdir)/python/ovs
>> +    touch $@
>> +CLEANFILES += pytest-check
>> +
>> +endif
>> diff --git a/python/ovs/tests/test_kv.py b/python/ovs/tests/test_kv.py
>> new file mode 100644
>> index 000000000..4b6eb0ce8
>> --- /dev/null
>> +++ b/python/ovs/tests/test_kv.py
>> @@ -0,0 +1,74 @@
>> +import pytest
>> +
>> +from ovs.flows.kv import KVParser, KeyValue
>> +
>> +
>> [email protected](
>> +    "input_data,expected",
>> +    [
>> +        (
>> +            (
>> +                "cookie=0x0, duration=147566.365s, table=0, n_packets=39, 
>> n_bytes=2574, idle_age=65534, hard_age=65534",  # noqa: E501
>> +                None,
>> +            ),
>> +            [
>> +                KeyValue("cookie", 0),
>> +                KeyValue("duration", "147566.365s"),
>> +                KeyValue("table", 0),
>> +                KeyValue("n_packets", 39),
>> +                KeyValue("n_bytes", 2574),
>> +                KeyValue("idle_age", 65534),
>> +                KeyValue("hard_age", 65534),
>> +            ],
>> +        ),
>> +        (
>> +            (
>> +                
>> "load:0x4->NXM_NX_REG13[],load:0x9->NXM_NX_REG11[],load:0x8->NXM_NX_REG12[],load:0x1->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],mod_dl_src:0a:58:a9:fe:00:02,resubmit(,8)",
>>   # noqa: E501
>> +
>
> Is there a reason for the empty line?
>
>> +                None,
>> +            ),
>> +            [
>> +                KeyValue("load", "0x4->NXM_NX_REG13[]"),
>> +                KeyValue("load", "0x9->NXM_NX_REG11[]"),
>> +                KeyValue("load", "0x8->NXM_NX_REG12[]"),
>> +                KeyValue("load", "0x1->OXM_OF_METADATA[]"),
>> +                KeyValue("load", "0x1->NXM_NX_REG14[]"),
>> +                KeyValue("mod_dl_src", "0a:58:a9:fe:00:02"),
>> +                KeyValue("resubmit", ",8"),
>> +            ],
>> +        ),
>> +        (("l1(l2(l3(l4())))", None), [KeyValue("l1", "l2(l3(l4()))")]),
>
> Maybe format this one the same as the others, making it easier to read!
>
>> +        (
>> +            ("l1(l2(l3(l4()))),foo:bar", None),
>> +            [KeyValue("l1", "l2(l3(l4()))"), KeyValue("foo", "bar")],
>> +        ),
>> +        (
>> +            ("enqueue:1:2,output=2", None),
>> +            [KeyValue("enqueue", "1:2"), KeyValue("output", 2)],
>> +        ),
>> +        (
>> +            ("value_to_reg(100)->someReg[10],foo:bar", None),
>> +            [
>> +                KeyValue("value_to_reg", "(100)->someReg[10]"),
>> +                KeyValue("foo", "bar"),
>> +            ],
>> +        ),
>> +    ],
>> +)
>> +def test_kv_parser(input_data, expected):
>> +    input_string = input_data[0]
>> +    decoders = input_data[1]
>> +    tparser = KVParser(decoders)
>> +    tparser.parse(input_string)
>
> Looking at this, I feel like the parse part should be part of the __init__ 
> function.
>
> tparser = KVParser(input_string, decoders)
>
>> +    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

Reply via email to