Patchwork tests: actually restore the original environment before running syshg

login
register
mail settings
Submitter Yuya Nishihara
Date June 30, 2017, 2:58 p.m.
Message ID <fe3419b41a456661a3d3.1498834708@mimosa>
Download mbox | patch
Permalink /patch/21848/
State Accepted
Headers show

Comments

Yuya Nishihara - June 30, 2017, 2:58 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1498826969 -32400
#      Fri Jun 30 21:49:29 2017 +0900
# Node ID fe3419b41a456661a3d35ae963d06a203d4446a2
# Parent  40ee74bfa11122a0a3ab74186f5056243b84af89
tests: actually restore the original environment before running syshg

Since os.environ may be overridden in run-tests.py, several important
variables such as PATH weren't restored.

I don't like the idea of using the system hg *by default* because the
executable and the configs are out of our control. But I don't mind as
long as the tests pass.
Adam Simpkins - June 30, 2017, 6:08 p.m.
On Jun 30, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1498826969 -32400
> #      Fri Jun 30 21:49:29 2017 +0900
> # Node ID fe3419b41a456661a3d35ae963d06a203d4446a2
> # Parent  40ee74bfa11122a0a3ab74186f5056243b84af89
> tests: actually restore the original environment before running syshg
> 
> Since os.environ may be overridden in run-tests.py, several important
> variables such as PATH weren't restored.
> 
> I don't like the idea of using the system hg *by default* because the
> executable and the configs are out of our control. But I don't mind as
> long as the tests pass.

Thanks for helping track down and fix the issue!  This diff looks good
to me.
Augie Fackler - July 3, 2017, 7:07 p.m.
On Fri, Jun 30, 2017 at 11:08:27AM -0700, Adam Simpkins wrote:
> On Jun 30, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1498826969 -32400
> > #      Fri Jun 30 21:49:29 2017 +0900
> > # Node ID fe3419b41a456661a3d35ae963d06a203d4446a2
> > # Parent  40ee74bfa11122a0a3ab74186f5056243b84af89
> > tests: actually restore the original environment before running syshg
> >
> > Since os.environ may be overridden in run-tests.py, several important
> > variables such as PATH weren't restored.
> >
> > I don't like the idea of using the system hg *by default* because the
> > executable and the configs are out of our control. But I don't mind as
> > long as the tests pass.
>
> Thanks for helping track down and fix the issue!  This diff looks good
> to me.


Queued, thanks

>
> --
> Adam Simpkins
> simpkins@fb.com
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -84,6 +84,7 @@  if os.environ.get('RTUNICODEPEDANTRY', F
     except NameError:
         pass
 
+origenviron = os.environ.copy()
 osenvironb = getattr(os, 'environb', os.environ)
 processlock = threading.Lock()
 
@@ -907,16 +908,21 @@  class Test(unittest.TestCase):
         # us to export.
         name_regex = re.compile('[a-zA-Z][a-zA-Z0-9_]*')
 
+        # Do not restore these variables; otherwise tests would fail.
+        reqnames = {'PYTHON', 'TESTDIR', 'TESTTMP'}
+
         with open(scriptpath, 'w') as envf:
-            for name, value in os.environ.items():
+            for name, value in origenviron.items():
                 if not name_regex.match(name):
                     # Skip environment variables with unusual names not
                     # allowed by most shells.
                     continue
+                if name in reqnames:
+                    continue
                 envf.write('%s=%s\n' % (name, shellquote(value)))
 
             for name in testenv:
-                if name in os.environ:
+                if name in origenviron or name in reqnames:
                     continue
                 envf.write('unset %s\n' % (name,))