On 3/11/22 16:21, Adrian Moreno wrote: > Some ovs-ofctl commands are used to parse or dump openflow flows, > specially in ofp-actions.at > > Use a wrapper around ovs-ofctl, called ovs-test-ofparse.py that, apart > from calling ovs-ofctl, also parses its output (or input, depending on > the command) to make sure the python flow parsing library can also parse > the flows.
Hi, Adrian. Since you're only touching the ofp-actions.at, it's probably better to just add a separate check for these rules. i.e. create a simpler script that just parses the set of flows without executing ovs-ofctl. I think, it will be much more intuitive for the readers of the test file. In most of the tests you already have the flow as a string or a set of flows in the file. You can sed out the 'bad' ones from these files pretty easily before feeding them to the script. What do you think? And you should add same checks to positive tests in ovs-ofctl.at, otherwise we're missing a big chunk of different matches. The idea of having a wrapper is OK, but for me it makes sense only if it's a global wrapper defined as an alias for all the ovs-ofctl invocations in the testsuite. And even then we could create an alias that will run 2 separate commands without invoking one from another. And since all new actions should be tested in ofp-actions.at and new matches in ovs-ofctl.at, it's probably OK to not have a global alias. > > Acked-by: Eelco Chaudron <[email protected]> > Signed-off-by: Adrian Moreno <[email protected]> > --- > tests/automake.mk | 3 ++ > tests/ofp-actions.at | 46 ++++++++-------- > tests/ovs-test-ofparse.py | 107 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 133 insertions(+), 23 deletions(-) > create mode 100755 tests/ovs-test-ofparse.py > > diff --git a/tests/automake.mk b/tests/automake.mk > index 8a9151f81..230085236 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -19,9 +19,11 @@ EXTRA_DIST += \ > $(OVSDB_CLUSTER_TESTSUITE) \ > tests/atlocal.in \ > $(srcdir)/package.m4 \ > + $(srcdir)/tests/ovs-test-ofparse.py \ > $(srcdir)/tests/testsuite \ > $(srcdir)/tests/testsuite.patch > > + No need for the extra line here. > COMMON_MACROS_AT = \ > tests/ovsdb-macros.at \ > tests/ovs-macros.at \ > @@ -523,6 +525,7 @@ CHECK_PYFILES = \ > tests/flowgen.py \ > tests/mfex_fuzzy.py \ > tests/ovsdb-monitor-sort.py \ > + tests/ovs-test-ofparse.py \ > tests/test-daemon.py \ > tests/test-json.py \ > tests/test-jsonrpc.py \ > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at > index 9d820eba6..6ee0d4773 100644 > --- a/tests/ofp-actions.at > +++ b/tests/ofp-actions.at > @@ -327,7 +327,7 @@ AT_CAPTURE_FILE([input.txt]) > AT_CAPTURE_FILE([expout]) > AT_CAPTURE_FILE([experr]) > AT_CHECK( > - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < > input.txt], > + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 > < input.txt], > [0], [expout], [experr]) Something like: AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < input.txt], [0], [expout], [experr]) AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
