On Wed, Jun 12, 2019 at 12:17 AM Eric Garver <eric@xxxxxxxxxxx> wrote:
>
> On Sun, Jun 09, 2019 at 11:48:49PM +0530, Shekhar Sharma wrote:
> > This patch adds the netns feature to the 'nft-test.py' file.
> >
> > Signed-off-by: Shekhar Sharma <shekhar250198@xxxxxxxxx>
> > ---
> > The version history of the patch is :
> > v1: add the netns feature
> > v2: use format() method to simplify print statements.
> > v3: updated the shebang
> > v4: resent the same with small changes
> >
> > tests/py/nft-test.py | 98 ++++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 80 insertions(+), 18 deletions(-)
> >
> > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> > index 4e18ae54..c9f65dc5 100755
> > --- a/tests/py/nft-test.py
> > +++ b/tests/py/nft-test.py
> [..]
> > @@ -245,6 +251,8 @@ def table_delete(table, filename=None, lineno=None):
> > return -1
> >
> > cmd = "delete table %s" % table
> > + if netns:
> > + cmd = "ip netns exec ___nftables-container-test {}".format(cmd)
>
> Can we pass the netns name via the netns argument? Then we don't have to
> have instances of ___nftables-container-test in the command literals.
OK.
> Just make netns part of your string format. You can also change the
> default arg value to "".
>
Should i keep the default value in all the occurrences (in those
functions as well where no other default arguments are used? )
> Please fix all occurrences.
>
> > ret = execute_cmd(cmd, filename, lineno)
> > if ret != 0:
> > reason = "%s: I cannot delete table %s. Giving up!" % (cmd, table)
> [..]
> > @@ -1208,6 +1262,9 @@ def run_test_file(filename, force_all_family_option, specific_file):
> > filename_path = os.path.join(TESTS_PATH, filename)
> > f = open(filename_path)
> > tests = passed = total_unit_run = total_warning = total_error = 0
> > + if netns:
> > + execute_cmd("ip netns add ___nftables-container-test", filename, 0)
> > +
>
> netns is not defined here.
>
> >
> > for lineno, line in enumerate(f):
> > sys.stdout.flush()
> > @@ -1327,6 +1384,8 @@ def run_test_file(filename, force_all_family_option, specific_file):
> > else:
> > if tests == passed and tests > 0:
> > print(filename + ": " + Colors.GREEN + "OK" + Colors.ENDC)
> > + if netns:
> > + execute_cmd("ip netns del ___nftables-container-test", filename, 0)
>
> netns is not defined here.
Sorry, will correct that.
Thanks!
Shekhar
