Merge ~ahasenack/qa-regression-testing:postfix-py3-fixes into qa-regression-testing:master

[Andreas Hasenack]

Remaining py3 fixes. I didn't change any defaults or package dependencies, i.e., if just following the README instructions, py2 will still be used.

I was also getting a lot of warnings about files left open, so I took a stab at that too.

In eoan:
$ sudo python3 ./test-postfix.py
...............
----------------------------------------------------------------------
Ran 15 tests in 62.400s

OK

$ sudo python2 ./test-postfix.py
...............
----------------------------------------------------------------------
Ran 15 tests in 63.094s

OK

I also checked xenial:
$ sudo python3 ./test-postfix.py
...............
----------------------------------------------------------------------
Ran 15 tests in 62.069s

OK
$ apt-cache policy python3
python3:
  Installed: 3.5.1-3
  Candidate: 3.5.1-3
  Version table:
 *** 3.5.1-3 500
        500 http://br.archive.ubuntu.com/ubuntu xenial/main amd64 Packages
        100 /var/lib/dpkg/status

[Steve Beattie (sbeattie)]

Looks good to me. Thanks for fixing the open file descriptor issues (ugh).

With your fixes, I verified that the tests continue to succeed on trusty and precise (ESM releases). Both worked under python2 and trusty did for python3; precise is missing a python3-pexpect package. That's not a blocker for merging this PR, but it complicates our plan to move to python3 by default for qrt scripts.

Thanks.

review: Approve

diff --git a/scripts/test-postfix.py b/scripts/test-postfix.py
index b294791..a616ba9 100755
--- a/scripts/test-postfix.py
+++ b/scripts/test-postfix.py
@@ -49,13 +49,13 @@ class PostfixTest(testlib.TestlibCase):
 
         # Move listener to localhost:2525
         conf_file = '/etc/postfix/master.cf'
-        lines = open(conf_file)
         contents = ''
-        for cfline in lines:
-            if cfline.startswith('smtp') and 'smtpd' in cfline and 'inet' in cfline:
-                contents += '127.0.0.1:2525      inet  n       -       -       -       -       smtpd\n'
-            else:
-                contents += "%s\n" % cfline
+        with open(conf_file) as fh:
+            for cfline in fh:
+                if cfline.startswith('smtp') and 'smtpd' in cfline and 'inet' in cfline:
+                    contents += '127.0.0.1:2525      inet  n       -       -       -       -       smtpd\n'
+                else:
+                    contents += "%s\n" % cfline
         testlib.config_replace(conf_file, contents, append=False)
 
         conf_file = '/etc/postfix/main.cf'
@@ -205,7 +205,7 @@ mech_list: %s %s
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         s.settimeout(5)
         s.connect(('localhost', 2525))
-        greeting = s.recv(1024)
+        greeting = s.recv(1024).decode('utf-8')
         # 220 gorgon.outflux.net ESMTP Postfix (Ubuntu)
         self.assertTrue(greeting.startswith('220 '), greeting)
         self.assertTrue('ESMTP' in greeting, greeting)
@@ -226,11 +226,11 @@ mech_list: %s %s
         reply = '%d %s' % (code, msg)
         if valid:
             self.assertEqual(code, 252, reply)
-            self.assertTrue(address in msg, reply)
+            self.assertTrue(address.encode('utf-8') in msg, reply)
         else:
             self.assertEqual(code, 550, reply)
-            self.assertTrue('Recipient address rejected' in msg, reply)
-            self.assertTrue('<%s>' % (address) in msg, reply)
+            self.assertTrue(b'Recipient address rejected' in msg, reply)
+            self.assertTrue(('<%s>' % (address)).encode('utf-8') in msg, reply)
 
     def test_10_commands(self):
         '''Basic SMTP commands'''
@@ -241,13 +241,13 @@ mech_list: %s %s
         reply = '%d %s' % (code, msg)
         self.assertEqual(code, 250, reply)
         self.assertEqual(self.s.does_esmtp, 1, reply)
-        self.assertTrue('8BITMIME' in self.s.ehlo_resp, reply)
+        self.assertTrue(b'8BITMIME' in self.s.ehlo_resp, reply)
         # No help available
         self.s.putcmd("help")
         code, msg = self.s.getreply()
         reply = '%d %s' % (code, msg)
         self.assertEqual(code, 502, reply)
-        self.assertTrue('Error' in msg, reply)
+        self.assertTrue(b'Error' in msg, reply)
         # VRFY addresses
         self._vrfy('[e-mail]', valid=True)
         self._vrfy('does-not-exist', valid=False)
@@ -286,7 +286,8 @@ Hello, nice to meet you.
         if spool_file == None:
             spool_file = '/var/mail/%s' % (target_spool_user.login)
         time.sleep(1)
-        contents = open(spool_file).read()
+        with open(spool_file) as fh:
+            contents = fh.read()
         #print contents
         # Server-side added headers...
         self.assertTrue('\nReceived: ' in contents, contents)
@@ -336,7 +337,8 @@ Hello, nice to meet you.
 
     def _write_forward(self, user, contents):
         forward_filename = '/home/%s/.forward' % (user.login)
-        open(forward_filename, 'w').write(contents)
+        with open(forward_filename, 'w') as fh:
+            fh.write(contents)
         os.chown(forward_filename, user.uid, user.gid)
 
     def test_10_sending_mail_forward_normal(self):
@@ -372,7 +374,8 @@ Hello, nice to meet you.
 
         # First, create our "target" file
         secret = '/root/secret.txt'
-        open(secret, 'w').write('Secret information\n')
+        with open(secret, 'w') as fh:
+            fh.write('Secret information\n')
         os.chmod(secret, 0o700)
 
         # Now, create a symlink to the target (we're going to use /var/tmp
@@ -398,7 +401,8 @@ Hello, nice to pwn you.
         # Pause for delivery
         time.sleep(2)
 
-        contents = open(secret).read()
+        with open(secret) as fh:
+            contents = fh.read()
         # Clean up before possible failures
         os.unlink('/var/mail/%s' % (self.user.login))
         os.unlink('/var/tmp/secret.link')
@@ -422,7 +426,7 @@ Hello, nice to pwn you.
         reply = '%d %s' % (code, msg)
         self.assertEqual(code, 250, reply)
         self.assertEqual(self.s.does_esmtp, 1, reply)
-        self.assertTrue('%s' % mech in self.s.ehlo_resp, reply)
+        self.assertTrue(mech.encode('utf-8') in self.s.ehlo_resp, reply)
         return reply
 
     def test_20_sasldb_cram_md5(self):