Merge lp:~widelands-dev/widelands/python3 into lp:widelands

[Tino]

Add a compatibility layer to regression_test.py to allow running with python 2 and 3

Add a compatibility layer to regression_test.py to allow running with python 2 and 3

[Klaus Halfmann (klaus-halfmann)]

Looks like you know more about python than I do,
lets check this ...

[Klaus Halfmann (klaus-halfmann)]

OK, thas as far as I came, too but now I get
$ ./regression_test.py -b ./widelands
...
File "./regression_test.py", line 103, in run_widelands
  stdout_file.write(line)
TypeError: must be str, not bytes

I used
   print(line, flush=True)
for
   stdout_file.write(line)
   stdout_file.flush()

but this in not python2.7 compatible :-(

In addition I used

< "Widelands exited abnormally. %s" % common_msg
---
> "Widelands exited abnormally. " + common_msg

< "Not all tests pass. %s." % common_msg
---
> "Not all tests pass. " + common_msg

but perhaos that .format() aproach is more compatible

[Klaus Halfmann (klaus-halfmann)]

Mhh, even using python 2.7 does not work, it just gets stuck in the splasg screeen?
Was there some change in the Lua Binding?

[Tino (tino79)]

Hm, I did run the test with both Python 2/3 without these errors, but perhaps I aborted them too early.
Or the test do not currently run on windows?

I'll check this later today...

review: Needs Fixing

[GunChleoc (gunchleoc)]

On my system. this will start the Widelands main screen for each test instead of going to the scenario. Python 2.7

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 980. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/121409569.
Appveyor build 813. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_python3-813.

[Tino (tino79)]

Ok, please have another look on linux.

review: Needs Resubmitting

[GunChleoc (gunchleoc)]

It's working for me now :)

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 985. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/121622180.
Appveyor build 818. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_python3-818.

[Klaus Halfmann (klaus-halfmann)]

$ ./regression_test.py -b ./widelands ; ## thats Python 3.4.4
Ran 32 tests in 1018.389s

$ python2.7 ./regression_test.py -b ./widelands
...
Ran 32 tests in 616.502s

Works but Python 3.4.4 was much slower.
Well I used this first so all files where cached.

So we had tests on windows, ubuntuntu and OSX, ready for merge?

review: Approve (review / test / compile)

[Tino (tino79)]

I cannot imagine why Python3 should be slower, because it is a very simple script and all test computing is done by the widelands binary. I am pretty sure if you would do another run with Python 3 it should be equally fast (perhaps your OS or your harddisk caches, or something other was occupying your CPU).

@bunnybot merge

review: Approve

=== modified file 'regression_test.py'
--- regression_test.py	2016-02-04 12:21:58 +0000
+++ regression_test.py	2016-04-08 06:11:28 +0000
@@ -12,6 +12,22 @@
 import unittest
 import platform
 
+#Python2/3 compat code for iterating items
+try:
+    dict.iteritems
+except AttributeError:
+    # Python 3
+    def itervalues(d):
+        return iter(d.values())
+    def iteritems(d):
+        return iter(d.items())
+else:
+    # Python 2
+    def itervalues(d):
+        return d.itervalues()
+    def iteritems(d):
+        return d.iteritems()
+
 def datadir():
     return os.path.join(os.path.dirname(__file__), "data")
 
@@ -63,20 +79,22 @@
         impact the filenames for stdout.txt.
 
         Returns the stdout filename."""
-        stdout_filename = os.path.join(self.run_dir, "stdout_%02i.txt" % which_time)
+        stdout_filename = os.path.join(self.run_dir, "stdout_{:02d}.txt".format(which_time))
+        log_filename = os.path.join(self.run_dir, "log_{:02d}.txt".format(which_time))
         if (os.path.exists(stdout_filename)):
             os.unlink(stdout_filename)
 
         with open(stdout_filename, 'a') as stdout_file:
             args = [self.path_to_widelands_binary,
                     '--verbose=true',
-                    '--datadir=%s' % datadir(),
-                    '--datadir_for_testing=%s' % datadir_for_testing(),
-                    '--homedir=%s' % self.run_dir,
+                    '--datadir={}'.format(datadir()),
+                    '--datadir_for_testing={}'.format(datadir_for_testing()),
+                    '--homedir={}'.format(self.run_dir),
                     '--disable_fx=true',
                     '--disable_music=true',
-                    '--language=en_US' ]
-            args += [ "--%s=%s" % (key, value) for key, value in wlargs.iteritems() ]
+                    '--language=en_US',
+                    '--logfile={}'.format(log_filename) ]
+            args += [ "--{}={}".format(key, value) for key, value in iteritems(wlargs) ]
 
             widelands = subprocess.Popen(
                     args, shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
@@ -84,21 +102,14 @@
                 line = widelands.stdout.readline()
                 if not line:
                     break
-                stdout_file.write(line)
+                stdout_file.write(str(line))
                 stdout_file.flush()
             widelands.communicate()
-            if platform.system() == "Windows":
-                 win_stdout = self.path_to_widelands_binary.replace("widelands.exe","stdout.txt")
-                 win_stderr = self.path_to_widelands_binary.replace("widelands.exe","stderr.txt")
-                 with open(win_stdout,"r") as f:
-                     for line in f:
-                         stdout_file.write(line)
-                     stdout_file.flush()
-                 if (os.path.exists(win_stderr)):
-                     with open(win_stderr,"r") as f:
-                         for line in f:
-                             stdout_file.write(line)
-                         stdout_file.flush()
+
+            with open(log_filename,"r") as f:
+                for line in f:
+                    stdout_file.write(line)
+                stdout_file.flush()
 
             self.widelands_returncode = widelands.returncode
         return stdout_filename
@@ -115,9 +126,9 @@
         while not all(savegame_done.values()):
             for savegame in sorted(savegame_done):
                 if not savegame_done[savegame]: break
-            out("  Loading savegame: %s ... " % savegame)
+            out("  Loading savegame: {} ... ".format(savegame))
             stdout_filename = self.run_widelands({ "loadgame": os.path.join(
-                self.run_dir, "save", "%s.wgf" % savegame) }, which_time)
+                self.run_dir, "save", "{}.wgf".format(savegame))}, which_time)
             which_time += 1
             stdout = open(stdout_filename, "r").read()
             for new_save in find_saves(stdout):
@@ -127,17 +138,17 @@
             self.verify_success(stdout, stdout_filename)
 
     def verify_success(self, stdout, stdout_filename):
-        common_msg = "Analyze the files in %s to see why this test case failed. Stdout is\n  %s\n\nstdout:\n%s" % (
+        common_msg = "Analyze the files in {} to see why this test case failed. Stdout is\n  {}\n\nstdout:\n{}".format(
                 self.run_dir, stdout_filename, stdout)
         self.assertTrue(self.widelands_returncode == 0,
-            "Widelands exited abnormally. %s" % common_msg
+            "Widelands exited abnormally. {}".format(common_msg)
         )
         self.assertTrue("All Tests passed" in stdout,
-            "Not all tests pass. %s." % common_msg
+            "Not all tests pass. {}.".format(common_msg)
         )
         out("done.\n")
         if self.keep_output_around:
-            out("    stdout: %s\n" % stdout_filename)
+            out("    stdout: {}\n".format(stdout_filename))
 
 def parse_args():
     p = argparse.ArgumentParser(description=
@@ -214,7 +225,7 @@
     args = parse_args()
 
     WidelandsTestCase.path_to_widelands_binary = args.binary
-    print "Using '%s' binary." % args.binary
+    print("Using '{}' binary.".format(args.binary)) 
     WidelandsTestCase.do_use_random_directory = not args.nonrandom
     WidelandsTestCase.keep_output_around = args.keep_around
 
@@ -222,7 +233,7 @@
     if args.regexp:
         all_files = [filename for filename in all_files if re.search(args.regexp, filename) ]
 
-    all_modules = [ "test.%s" % filename[:-3] for filename in all_files ]
+    all_modules = [ "test.{}".format(filename[:-3]) for filename in all_files ]
 
     suite = unittest.TestSuite()
     discover_loadgame_tests(args.regexp, suite)