| Message ID | 20211122112256.2011194-15-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | python: add flow parsing library | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 22 Nov 2021, at 12:22, Adrian Moreno wrote: > Use pytest to run unit tests > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > .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 = \ > + 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 > + > + > +@pytest.mark.parametrize( > + "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
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 <amorenoz@redhat.com> >> --- >> .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 >> + >> + >> +@pytest.mark.parametrize( >> + "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
On 12/24/21 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 <amorenoz@redhat.com> >> --- >> .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? > Sure. Sill do. >> 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? > Will do. Thanks. >> /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 = \ >> + 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 >> + >> + >> +@pytest.mark.parametrize( >> + "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) > Yep. Will change it. >> + 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 >
Hi, >>> 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: Just FYI, In the new version, I'm changing this macro to check for all test dependencies.
On 1/26/22 13:34, Adrian Moreno wrote: > Hi, > >>>> 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: > > > Just FYI, In the new version, I'm changing this macro to check for all test dependencies. > Is pytest a build dependency or a test dependency? If it's only for testing, the check should, probably, be part of the tests/atlocal.in, which is executed every time before starting the testsuite. Best regards, Ilya Maximets.
On 1/26/22 13:43, Ilya Maximets wrote: > On 1/26/22 13:34, Adrian Moreno wrote: >> Hi, >> >>>>> 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: >> >> >> Just FYI, In the new version, I'm changing this macro to check for all test dependencies. >> > > Is pytest a build dependency or a test dependency? > If it's only for testing, the check should, probably, > be part of the tests/atlocal.in, which is executed > every time before starting the testsuite. > Hi Ilya, that's a good question. It's for testing but since I was mimicking the flake8 tests, I have included them in the ALL_LOCAL target. But I think it'd better if they are run as part of "make check", right? > Best regards, Ilya Maximets. >
On 1/26/22 14:33, Adrian Moreno wrote: > > > On 1/26/22 13:43, Ilya Maximets wrote: >> On 1/26/22 13:34, Adrian Moreno wrote: >>> Hi, >>> >>>>>> 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: >>> >>> >>> Just FYI, In the new version, I'm changing this macro to check for all test dependencies. >>> >> >> Is pytest a build dependency or a test dependency? >> If it's only for testing, the check should, probably, >> be part of the tests/atlocal.in, which is executed >> every time before starting the testsuite. >> > > Hi Ilya, that's a good question. It's for testing but since I was > mimicking the flake8 tests, I have included them in the ALL_LOCAL > target. But I think it'd better if they are run as part of "make check", > right? Yeah, flake8 is just a style check, while these are actual unit tests, so should belong to a testsuite, I think. > >> Best regards, Ilya Maximets. >> >
On 1/26/22 14:45, Ilya Maximets wrote: > On 1/26/22 14:33, Adrian Moreno wrote: >> >> >> On 1/26/22 13:43, Ilya Maximets wrote: >>> On 1/26/22 13:34, Adrian Moreno wrote: >>>> Hi, >>>> >>>>>>> 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: >>>> >>>> >>>> Just FYI, In the new version, I'm changing this macro to check for all test dependencies. >>>> >>> >>> Is pytest a build dependency or a test dependency? >>> If it's only for testing, the check should, probably, >>> be part of the tests/atlocal.in, which is executed >>> every time before starting the testsuite. >> >> >> Hi Ilya, that's a good question. It's for testing but since I was >> mimicking the flake8 tests, I have included them in the ALL_LOCAL >> target. But I think it'd better if they are run as part of "make check", >> right? > > Yeah, flake8 is just a style check, while these are actual unit tests, > so should belong to a testsuite, I think. > Do you think it's better to add a new test as part of "testsuite.at" or create a new "python-testsuite.at" and a new target, e.g: "make check-python"? A benefit of the former is that it'll be integrated into the CI for free. A benefit from the latter is that it'll be faster since there is no need to build the entire project. >> >>> Best regards, Ilya Maximets. >>> >> >
On 1/27/22 15:44, Adrian Moreno wrote: > > > On 1/26/22 14:45, Ilya Maximets wrote: >> On 1/26/22 14:33, Adrian Moreno wrote: >>> >>> >>> On 1/26/22 13:43, Ilya Maximets wrote: >>>> On 1/26/22 13:34, Adrian Moreno wrote: >>>>> Hi, >>>>> >>>>>>>> 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: >>>>> >>>>> >>>>> Just FYI, In the new version, I'm changing this macro to check for all test dependencies. >>>>> >>>> >>>> Is pytest a build dependency or a test dependency? >>>> If it's only for testing, the check should, probably, >>>> be part of the tests/atlocal.in, which is executed >>>> every time before starting the testsuite. >>> > >>> >>> Hi Ilya, that's a good question. It's for testing but since I was >>> mimicking the flake8 tests, I have included them in the ALL_LOCAL >>> target. But I think it'd better if they are run as part of "make check", >>> right? >> >> Yeah, flake8 is just a style check, while these are actual unit tests, >> so should belong to a testsuite, I think. >> > > Do you think it's better to add a new test as part of "testsuite.at" or create a new "python-testsuite.at" and a new target, e.g: "make check-python"? > > A benefit of the former is that it'll be integrated into the CI for free. > A benefit from the latter is that it'll be faster since there is no need to build the entire project. We're usually creating a special testsuite only if it's very inconvenient to run these tests as part of the default one. For example, system tests needs root privileges and modifies your system environment, ovsdb-cluster tests are very heavy and require a lot of time and CPU resources to be useful. These python tests doesn't seem disruptive or heavy, so it's better to run them by default on all platforms. Best regards, Ilya Maximets.
On 1/28/22 13:40, Ilya Maximets wrote: > On 1/27/22 15:44, Adrian Moreno wrote: >> >> >> On 1/26/22 14:45, Ilya Maximets wrote: >>> On 1/26/22 14:33, Adrian Moreno wrote: >>>> >>>> >>>> On 1/26/22 13:43, Ilya Maximets wrote: >>>>> On 1/26/22 13:34, Adrian Moreno wrote: >>>>>> Hi, >>>>>> >>>>>>>>> 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: >>>>>> >>>>>> >>>>>> Just FYI, In the new version, I'm changing this macro to check for all test dependencies. >>>>>> >>>>> >>>>> Is pytest a build dependency or a test dependency? >>>>> If it's only for testing, the check should, probably, >>>>> be part of the tests/atlocal.in, which is executed >>>>> every time before starting the testsuite. >>>> >> >>>> >>>> Hi Ilya, that's a good question. It's for testing but since I was >>>> mimicking the flake8 tests, I have included them in the ALL_LOCAL >>>> target. But I think it'd better if they are run as part of "make check", >>>> right? >>> >>> Yeah, flake8 is just a style check, while these are actual unit tests, >>> so should belong to a testsuite, I think. >>> >> >> Do you think it's better to add a new test as part of "testsuite.at" or create a new "python-testsuite.at" and a new target, e.g: "make check-python"? >> >> A benefit of the former is that it'll be integrated into the CI for free. >> A benefit from the latter is that it'll be faster since there is no need to build the entire project. > > We're usually creating a special testsuite only if it's very inconvenient > to run these tests as part of the default one. For example, system tests > needs root privileges and modifies your system environment, ovsdb-cluster > tests are very heavy and require a lot of time and CPU resources to be > useful. > > These python tests doesn't seem disruptive or heavy, so it's better to run > them by default on all platforms. > Understood, thanks Ilya.
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 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 /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 + 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 = \ + 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 + + +@pytest.mark.parametrize( + "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 + + 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()))")]), + ( + ("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) + 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
Use pytest to run unit tests Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- .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