On Fri, Jun 7, 2019 at 8:18 AM Davide Caratti <dcara...@redhat.com> wrote: > From what I see, it is a fix for the reported problem (e.g. tests failing > because of 'nsPlugin' uninstalled). And, I want to followup fixing the > bpf.json in tc-actions, so that > > # ./tdc.py -l -c bpf | grep eBPF > e939: (actions, bpf) Add eBPF action with valid object-file > 282d: (actions, bpf) Add eBPF action with invalid object-file > > require the buildebpfPlugin (unless anybody disagrees, I will also revert > the meaning of '-B' also, like you did for '-n') Yes, those should definitely be tagged. I probably should have included them in the patch.
As for the -B option, that would probably be a good idea. Required plugins, I think, should always be on and require a user to explicitly not use them. So in that case, nsPlugin and buildepfPlugin, being required, should have options to explicitly disable their features. valgrindPlugin, since it's not explicitly required to run any given test, doesn't need that. Maybe we should separate the plugins into different directories based on this? > > few comments after a preliminary test: > 1) the patch still does not cover the two categories that use $DEV2 (i.e. > flower and concurrency still fail in my environment) I will have to try that out and see what is going on. I didn't try the $DEV2 tests against this. > 2) I've been reported, and reproduced with latest fedora, a problem in > nsPlugin.py. All tests in the 'filter' category still fail, unless I do > > # sed -i "s#ip#/sbin/ip#g" nsPlugin.py > > otherwise, the 'prepare' stage fails: > > # ./tdc.py -e 5339 > -- ns/SubPlugin.__init__ > Test 5339: Del entire fw filter > > -----> prepare stage *** Could not execute: "$TC qdisc add dev $DEV1 ingress" > > -----> prepare stage *** Error message: "/bin/sh: ip: command not found > " > returncode 127; expected [0] > > -----> prepare stage *** Aborting test run. > > (maybe we should use a variable for that, instead of hardcoded command > name, like we do for $TC ?) Ooh... Yes, yes we should. Sounds like the new version of Fedora isn't including sbin on the path anymore? That's going to require another minor change that I think I'll submit in a separate patch. Thanks, Davide!