Patchwork [3,of,3] py3: allow run-tests.py to run on Windows

login
register
mail settings
Submitter Matt Harbison
Date Sept. 15, 2018, 4:31 a.m.
Message ID <757f2fe237ea1f72d75f.1536985897@Envy>
Download mbox | patch
Permalink /patch/34688/
State Accepted
Headers show

Comments

Matt Harbison - Sept. 15, 2018, 4:31 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1536984246 14400
#      Sat Sep 15 00:04:06 2018 -0400
# Node ID 757f2fe237ea1f72d75ff29bd1bb428db38813be
# Parent  289c7157ec4510c619e9e7fdd9ac4995e13ce73e
py3: allow run-tests.py to run on Windows

This is now functional:

    HGMODULEPOLICY=py py -3 run-tests.py --local test-help.t --pure --view bcompare

However, on this machine without a C compiler, it tries to load cext anyway, and
blows up.  I haven't looked into why, other than to see that it does set the
environment variable.  When the test exits though, I see it can't find
killdaemons.py, get-with-headers.py, etc.

I have no idea why these changes are needed, given that it runs on Linux.  But
os.system() is insisting that it take a str, and subprocess.Popen() blows up
without str:

    Errored test-help.t: Traceback (most recent call last):
      File "run-tests.py", line 810, in run
        self.runTest()
      File "run-tests.py", line 858, in runTest
        ret, out = self._run(env)
      File "run-tests.py", line 1268, in _run
        exitcode, output = self._runcommand(cmd, env)
      File "run-tests.py", line 1141, in _runcommand
        env=env)
      File "C:\Program Files\Python37\lib\subprocess.py", line 756, in __init__
        restore_signals, start_new_session)
      File "C:\Program Files\Python37\lib\subprocess.py", line 1100, in _execute_child
        args = list2cmdline(args)
      File "C:\Program Files\Python37\lib\subprocess.py", line 511, in list2cmdline
        needquote = (" " in arg) or ("\t" in arg) or not arg
    TypeError: argument of type 'int' is not iterable

This is exactly how it crashes when trying to spin up a pager too.  I left one
instance of os.system() unchanged in _installhg(), because it doesn't get there.
Yuya Nishihara - Sept. 15, 2018, 6:23 a.m.
On Sat, 15 Sep 2018 00:31:37 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1536984246 14400
> #      Sat Sep 15 00:04:06 2018 -0400
> # Node ID 757f2fe237ea1f72d75ff29bd1bb428db38813be
> # Parent  289c7157ec4510c619e9e7fdd9ac4995e13ce73e
> py3: allow run-tests.py to run on Windows

Queued with some check-code fixes, thanks.

> I have no idea why these changes are needed, given that it runs on Linux.  But
> os.system() is insisting that it take a str, and subprocess.Popen() blows up
> without str:

That's probably because a byte string is the native data structure on Unix.
Unicode story of Python 3 is horrible in many ways.
Matt Harbison - Sept. 18, 2018, 4:04 a.m.
On Sat, 15 Sep 2018 00:31:37 -0400, Matt Harbison <mharbison72@gmail.com>  
wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1536984246 14400
> #      Sat Sep 15 00:04:06 2018 -0400
> # Node ID 757f2fe237ea1f72d75ff29bd1bb428db38813be
> # Parent  289c7157ec4510c619e9e7fdd9ac4995e13ce73e
> py3: allow run-tests.py to run on Windows
>
> This is now functional:
>
>     HGMODULEPOLICY=py py -3 run-tests.py --local test-help.t --pure  
> --view bcompare
>
> However, on this machine without a C compiler, it tries to load cext  
> anyway, and
> blows up.  I haven't looked into why, other than to see that it does set  
> the
> environment variable.  When the test exits though, I see it can't find
> killdaemons.py, get-with-headers.py, etc.

It looks like part of the problem with not finding these files is TESTDIR  
is not defined when running the *.t file.  Inside Test.runTest(),  
'TESTDIR' is not in env, osenvironb, or os.environ on py3 (but it is on  
py2).  I'm assuming this is a bytes vs unicode thing since Linux works,  
but I can't tell where it is being set in the first place.  The one place  
it is set around run-tests.py:2511 is skipped on both py2 and py3 when a  
relative *.t is given.  A few other variables are missing too, like  
PYTHON.  The HG* variables from _getenv() are defined.
Yuya Nishihara - Sept. 18, 2018, 1:40 p.m.
> It looks like part of the problem with not finding these files is TESTDIR  
> is not defined when running the *.t file.  Inside Test.runTest(),  
> 'TESTDIR' is not in env, osenvironb, or os.environ on py3 (but it is on  
> py2).  I'm assuming this is a bytes vs unicode thing since Linux works,  
> but I can't tell where it is being set in the first place.  The one place  
> it is set around run-tests.py:2511 is skipped on both py2 and py3 when a  
> relative *.t is given.  A few other variables are missing too, like  
> PYTHON.  The HG* variables from _getenv() are defined.

Perhaps, the real os.environ wouldn't be updated on Windows since environb
is a copy. We'll have to ditch implicit updates of the environ/environb.
Another option is to write an environb-like wrapper over os.environ.
Matt Harbison - Sept. 19, 2018, 1:51 a.m.
On Tue, 18 Sep 2018 09:40:58 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

>> It looks like part of the problem with not finding these files is  
>> TESTDIR
>> is not defined when running the *.t file.  Inside Test.runTest(),
>> 'TESTDIR' is not in env, osenvironb, or os.environ on py3 (but it is on
>> py2).  I'm assuming this is a bytes vs unicode thing since Linux works,
>> but I can't tell where it is being set in the first place.  The one  
>> place
>> it is set around run-tests.py:2511 is skipped on both py2 and py3 when a
>> relative *.t is given.  A few other variables are missing too, like
>> PYTHON.  The HG* variables from _getenv() are defined.
>
> Perhaps, the real os.environ wouldn't be updated on Windows since  
> environb
> is a copy. We'll have to ditch implicit updates of the environ/environb.
> Another option is to write an environb-like wrapper over os.environ.

You're right.  I forgot the b'' when fetching from osenvironb, so it  
didn't look set there either.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -229,7 +229,8 @@  def checkportisavailable(port):
 closefds = os.name == 'posix'
 def Popen4(cmd, wd, timeout, env=None):
     processlock.acquire()
-    p = subprocess.Popen(cmd, shell=True, bufsize=-1, cwd=wd, env=env,
+    p = subprocess.Popen(_strpath(cmd), shell=True, bufsize=-1,
+                         cwd=_strpath(wd), env=env,
                          close_fds=closefds,
                          stdin=subprocess.PIPE, stdout=subprocess.PIPE,
                          stderr=subprocess.STDOUT)
@@ -988,7 +989,7 @@  class Test(unittest.TestCase):
             return (
                 (b''.join(c.isalpha() and b'[%s%s]' % (c.lower(), c.upper()) or
                     c in b'/\\' and br'[/\\]' or c.isdigit() and c or b'\\' + c
-                    for c in p))
+                    for c in [p[i:i+1] for i in range(len(p))]))
             )
         else:
             return re.escape(p)
@@ -1136,7 +1137,8 @@  class Test(unittest.TestCase):
         Return a tuple (exitcode, output). output is None in debug mode.
         """
         if self._debug:
-            proc = subprocess.Popen(cmd, shell=True, cwd=self._testtmp,
+            proc = subprocess.Popen(_strpath(cmd), shell=True,
+                                    cwd=_strpath(self._testtmp),
                                     env=env)
             ret = proc.wait()
             return (ret, None)
@@ -1816,10 +1818,8 @@  class TestResult(unittest._TextTestResul
                 pass
             elif self._options.view:
                 v = self._options.view
-                if PYTHON3:
-                    v = _bytespath(v)
-                os.system(b"%s %s %s" %
-                          (v, test.refpath, test.errpath))
+                os.system(r"%s %s %s" %
+                          (v, _strpath(test.refpath), _strpath(test.errpath)))
             else:
                 servefail, lines = getdiff(expected, got,
                                            test.refpath, test.errpath)
@@ -2887,7 +2887,10 @@  class TestRunner(object):
         """Configure the environment to use the appropriate Python in tests."""
         # Tests must use the same interpreter as us or bad things will happen.
         pyexename = sys.platform == 'win32' and b'python.exe' or b'python'
-        if getattr(os, 'symlink', None):
+
+        # os.symlink() is a thing with py3 on Windows, but it requires
+        # Administrator rights.
+        if getattr(os, 'symlink', None) and os.name != 'nt':
             vlog("# Making python executable in test path a symlink to '%s'" %
                  sys.executable)
             mypython = os.path.join(self._tmpbindir, pyexename)