[5/6] test: make crit-recode.py python2/python3 compatible

Submitted by Andrey Vagin on May 30, 2018, 6:39 p.m.

Details

Message ID 20180530183939.32310-6-avagin@virtuozzo.com
State New
Series "More changes to be compatible with python 3"
Headers show

Commit Message

Andrey Vagin May 30, 2018, 6:39 p.m.
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
---
 test/crit-recode.py | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

Patch hide | download patch | download mbox

diff --git a/test/crit-recode.py b/test/crit-recode.py
index 0d4b31777..0cb3341d3 100755
--- a/test/crit-recode.py
+++ b/test/crit-recode.py
@@ -1,9 +1,11 @@ 
-#!/bin/env python2
+#!/usr/bin/env python
+# vim: noet ts=8 sw=8 sts=8
 
 import pycriu
 import sys
 import os
 import subprocess
+import json
 
 find = subprocess.Popen(['find', 'test/dump/', '-size', '+0', '-name', '*.img'],
 		stdout = subprocess.PIPE)
@@ -14,20 +16,21 @@  def recode_and_check(imgf, o_img, pretty):
 	try:
 		pb = pycriu.images.loads(o_img, pretty)
 	except pycriu.images.MagicException as me:
-		print "%s magic %x error" % (imgf, me.magic)
+		print("%s magic %x error" % (imgf, me.magic))
 		return False
-	except:
-		print "%s %sdecode fails" % (imgf, pretty and 'pretty ' or '')
+	except Exception as e:
+		print("%s %sdecode fails: %s" % (imgf, pretty and 'pretty ' or '', e))
 		return False
 
 	try:
 		r_img = pycriu.images.dumps(pb)
-	except:
-		print "%s %sencode fails" % (imgf, pretty and 'pretty ' or '')
+	except Exception as e:
+		r_img = pycriu.images.dumps(pb)
+		print("%s %s encode fails: %s" % (imgf, pretty and 'pretty ' or '', e))
 		return False
 
 	if o_img != r_img:
-		print "%s %srecode mismatch" % (imgf, pretty and 'pretty ' or '')
+		print("%s %s recode mismatch" % (imgf, pretty and 'pretty ' or ''))
 		return False
 
 	return True
@@ -37,28 +40,28 @@  for imgf in find.stdout.readlines():
 	imgf = imgf.strip()
 	imgf_b = os.path.basename(imgf)
 
-	if imgf_b.startswith('pages-'):
+	if imgf_b.startswith(b'pages-'):
 		continue
-	if imgf_b.startswith('iptables-'):
+	if imgf_b.startswith(b'iptables-'):
 		continue
-	if imgf_b.startswith('ip6tables-'):
+	if imgf_b.startswith(b'ip6tables-'):
 		continue
-	if imgf_b.startswith('route-'):
+	if imgf_b.startswith(b'route-'):
 		continue
-	if imgf_b.startswith('route6-'):
+	if imgf_b.startswith(b'route6-'):
 		continue
-	if imgf_b.startswith('ifaddr-'):
+	if imgf_b.startswith(b'ifaddr-'):
 		continue
-	if imgf_b.startswith('tmpfs-'):
+	if imgf_b.startswith(b'tmpfs-'):
 		continue
-	if imgf_b.startswith('netns-ct-'):
+	if imgf_b.startswith(b'netns-ct-'):
 		continue
-	if imgf_b.startswith('netns-exp-'):
+	if imgf_b.startswith(b'netns-exp-'):
 		continue
-	if imgf_b.startswith('rule-'):
+	if imgf_b.startswith(b'rule-'):
 		continue
 
-	o_img = open(imgf).read()
+	o_img = open(imgf.decode(), "rb").read()
 	if not recode_and_check(imgf, o_img, False):
 		test_pass = False
 	if not recode_and_check(imgf, o_img, True):
@@ -67,7 +70,7 @@  for imgf in find.stdout.readlines():
 find.wait()
 
 if not test_pass:
-	print "FAIL"
+	print("FAIL")
 	sys.exit(1)
 
-print "PASS"
+print("PASS")

Comments

Adrian Reber May 30, 2018, 7:09 p.m.
On Wed, May 30, 2018 at 09:39:38PM +0300, Andrei Vagin wrote:
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  test/crit-recode.py | 43 +++++++++++++++++++++++--------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/test/crit-recode.py b/test/crit-recode.py
> index 0d4b31777..0cb3341d3 100755
> --- a/test/crit-recode.py
> +++ b/test/crit-recode.py
> @@ -1,9 +1,11 @@
> -#!/bin/env python2
> +#!/usr/bin/env python

I understand what and why you are doing this but I just wanted to
mention that this is rather the opposite from what I heard should be
done. I am not a python expert so I am just repeating what I heard
elsewhere and I think the Fedora packaging guidelines are also
mentioning it.

It is better to explicitly use python2 or python3, especially as I
understand it, the python binary should never be python3 and one should
avoid the use of an unversioned python binary.

I understand that it makes it easier to run it in travis either with
python2 or python3 by changing the link like you did in the other
commit. Maybe our scripts should not include a shebang at all and always
be called with the corresponding interpreter. So either 'python2
test/crit-recode.py' or 'python3 test/crit-recode.py'.

This is in no way important to me, I just wanted to mention it as I had
the same problem with crit.

		Adrian
Andrey Vagin June 1, 2018, 10:24 p.m.
On Wed, May 30, 2018 at 09:09:42PM +0200, Adrian Reber wrote:
> On Wed, May 30, 2018 at 09:39:38PM +0300, Andrei Vagin wrote:
> > Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> > ---
> >  test/crit-recode.py | 43 +++++++++++++++++++++++--------------------
> >  1 file changed, 23 insertions(+), 20 deletions(-)
> > 
> > diff --git a/test/crit-recode.py b/test/crit-recode.py
> > index 0d4b31777..0cb3341d3 100755
> > --- a/test/crit-recode.py
> > +++ b/test/crit-recode.py
> > @@ -1,9 +1,11 @@
> > -#!/bin/env python2
> > +#!/usr/bin/env python
> 
> I understand what and why you are doing this but I just wanted to
> mention that this is rather the opposite from what I heard should be
> done. I am not a python expert so I am just repeating what I heard
> elsewhere and I think the Fedora packaging guidelines are also
> mentioning it.
> 
> It is better to explicitly use python2 or python3, especially as I
> understand it, the python binary should never be python3 and one should
> avoid the use of an unversioned python binary.
> 
> I understand that it makes it easier to run it in travis either with
> python2 or python3 by changing the link like you did in the other
> commit. Maybe our scripts should not include a shebang at all and always
> be called with the corresponding interpreter. So either 'python2
> test/crit-recode.py' or 'python3 test/crit-recode.py'.
> 
> This is in no way important to me, I just wanted to mention it as I had
> the same problem with crit.

I understand your points and I'm agree that we need to follow them for
crit, because it is a public tool. For test tools like zdtm.py and other
test scripts, I think we can avoid these tricks before we meet any
problem. Let me know if you don't afree with this.

> 
> 		Adrian
Adrian Reber June 2, 2018, 7:04 a.m.
On Fri, Jun 01, 2018 at 03:24:14PM -0700, Andrei Vagin wrote:
> On Wed, May 30, 2018 at 09:09:42PM +0200, Adrian Reber wrote:
> > On Wed, May 30, 2018 at 09:39:38PM +0300, Andrei Vagin wrote:
> > > Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> > > ---
> > >  test/crit-recode.py | 43 +++++++++++++++++++++++--------------------
> > >  1 file changed, 23 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/test/crit-recode.py b/test/crit-recode.py
> > > index 0d4b31777..0cb3341d3 100755
> > > --- a/test/crit-recode.py
> > > +++ b/test/crit-recode.py
> > > @@ -1,9 +1,11 @@
> > > -#!/bin/env python2
> > > +#!/usr/bin/env python
> > 
> > I understand what and why you are doing this but I just wanted to
> > mention that this is rather the opposite from what I heard should be
> > done. I am not a python expert so I am just repeating what I heard
> > elsewhere and I think the Fedora packaging guidelines are also
> > mentioning it.
> > 
> > It is better to explicitly use python2 or python3, especially as I
> > understand it, the python binary should never be python3 and one should
> > avoid the use of an unversioned python binary.
> > 
> > I understand that it makes it easier to run it in travis either with
> > python2 or python3 by changing the link like you did in the other
> > commit. Maybe our scripts should not include a shebang at all and always
> > be called with the corresponding interpreter. So either 'python2
> > test/crit-recode.py' or 'python3 test/crit-recode.py'.
> > 
> > This is in no way important to me, I just wanted to mention it as I had
> > the same problem with crit.
> 
> I understand your points and I'm agree that we need to follow them for
> crit, because it is a public tool. For test tools like zdtm.py and other
> test scripts, I think we can avoid these tricks before we meet any
> problem. Let me know if you don't afree with this.

That sounds like a good approach how to handle python2/python3 within
CRIU.

		Adrian