Merge pull request #2797 from xonsh/swapexcept

fixed environment swapping issue

news/swapexcept.rst

@@ -0,0 +1,14 @@
+**Added:** None
+
+**Changed:** None
+
+**Deprecated:** None
+
+**Removed:** None
+
+**Fixed:**
+
+* Environment swapping would not properly reraise errors due to weird
+  Python name binding issue.
+
+**Security:** None

tests/test_lib/test_subprocess.xsh

@@ -1,7 +1,10 @@
+"""Tests for subprocess lib"""
 import tempfile
 
 from xonsh.lib.os import indir
-from xonsh.lib.subprocess import run, check_call, check_output
+from xonsh.lib.subprocess import run, check_call, check_output, CalledProcessError
+
+from tools import skip_if_on_windows
 
 
 def test_run():
@@ -26,6 +29,16 @@ def test_check_call():
             assert 'tst_dir/hello.txt' in g`tst_dir/*.txt`
 
 
+@skip_if_on_windows
+def test_check_call_raises():
+    try:
+        check_call('false')
+        got_raise = False
+    except CalledProcessError:
+        got_raise = True
+    assert got_raise
+
+
 def test_check_output():
     with tempfile.TemporaryDirectory() as tmpdir:
         with indir(tmpdir):
@@ -36,3 +49,4 @@ def test_check_output():
             p = check_output(['touch', 'hello.txt'], cwd='tst_dir')
             assert p.decode('utf-8') == ''
             assert 'tst_dir/hello.txt' in g`tst_dir/*.txt`
+

tests/test_lib/tools.py

@@ -0,0 +1 @@
+../tools.py

tests/tools.py

@@ -23,46 +23,51 @@ VER_3_5 = (3, 5)
 VER_3_6 = (3, 6)
 VER_MAJOR_MINOR = sys.version_info[:2]
 VER_FULL = sys.version_info[:3]
-ON_DARWIN = (platform.system() == 'Darwin')
-ON_WINDOWS = (platform.system() == 'Windows')
-ON_MSYS = (sys.platform == 'msys')
-ON_CONDA = True in [conda in pytest.__file__.lower() for conda
-                    in ['conda', 'anaconda', 'miniconda']]
-ON_TRAVIS = 'TRAVIS' in os.environ and 'CI' in os.environ
+ON_DARWIN = platform.system() == "Darwin"
+ON_WINDOWS = platform.system() == "Windows"
+ON_MSYS = sys.platform == "msys"
+ON_CONDA = True in [
+    conda in pytest.__file__.lower() for conda in ["conda", "anaconda", "miniconda"]
+]
+ON_TRAVIS = "TRAVIS" in os.environ and "CI" in os.environ
 TEST_DIR = os.path.dirname(__file__)
 
 # pytest skip decorators
 skip_if_py34 = pytest.mark.skipif(VER_MAJOR_MINOR < VER_3_5, reason="Py3.5+ only test")
-skip_if_lt_py36 = pytest.mark.skipif(VER_MAJOR_MINOR < VER_3_6, reason="Py3.6+ only test")
+skip_if_lt_py36 = pytest.mark.skipif(
+    VER_MAJOR_MINOR < VER_3_6, reason="Py3.6+ only test"
+)
 
-skip_if_on_conda = pytest.mark.skipif(ON_CONDA,
-                        reason="Conda and virtualenv _really_ hate each other")
+skip_if_on_conda = pytest.mark.skipif(
+    ON_CONDA, reason="Conda and virtualenv _really_ hate each other"
+)
 
-skip_if_on_msys = pytest.mark.skipif(ON_MSYS,
-                        reason="MSYS and virtualenv _really_ hate each other")
+skip_if_on_msys = pytest.mark.skipif(
+    ON_MSYS, reason="MSYS and virtualenv _really_ hate each other"
+)
 
-skip_if_on_windows = pytest.mark.skipif(ON_WINDOWS, reason='Unix stuff')
+skip_if_on_windows = pytest.mark.skipif(ON_WINDOWS, reason="Unix stuff")
 
-skip_if_on_unix = pytest.mark.skipif(not ON_WINDOWS, reason='Windows stuff')
+skip_if_on_unix = pytest.mark.skipif(not ON_WINDOWS, reason="Windows stuff")
 
-skip_if_on_darwin = pytest.mark.skipif(ON_DARWIN, reason='not Mac friendly')
+skip_if_on_darwin = pytest.mark.skipif(ON_DARWIN, reason="not Mac friendly")
 
-skip_if_on_travis = pytest.mark.skipif(ON_TRAVIS, reason='not Travis CI friendly')
+skip_if_on_travis = pytest.mark.skipif(ON_TRAVIS, reason="not Travis CI friendly")
 
-skip_if_lt_ptk2 = pytest.mark.skipif(ptk_version_info()[0] < 2,
-                                     reason="prompt-tollkit <2")
+skip_if_lt_ptk2 = pytest.mark.skipif(
+    ptk_version_info()[0] < 2, reason="prompt-tollkit <2"
+)
 
 
 def sp(cmd):
     return subprocess.check_output(cmd, universal_newlines=True)
 
 
-class DummyStyler():
+class DummyStyler:
     styles = defaultdict(str)
 
 
 class DummyBaseShell(BaseShell):
-
     def __init__(self):
         self.styler = DummyStyler()
 
@@ -81,9 +86,8 @@ class DummyShell:
 
 
 class DummyCommandsCache:
-
     def locate_binary(self, name):
-        return os.path.join(os.path.dirname(__file__), 'bin', name)
+        return os.path.join(os.path.dirname(__file__), "bin", name)
 
     def predict_threadable(self, cmd):
         return True
@@ -92,7 +96,7 @@ class DummyCommandsCache:
 class DummyHistory:
 
     last_cmd_rtn = 0
-    last_cmd_out = ''
+    last_cmd_out = ""
 
     def append(self, x):
         pass
@@ -103,10 +107,7 @@ class DummyHistory:
 
 class DummyEnv(MutableMapping):
 
-    DEFAULTS = {
-        'XONSH_DEBUG': 1,
-        'XONSH_COLOR_STYLE': 'default',
-    }
+    DEFAULTS = {"XONSH_DEBUG": 1, "XONSH_COLOR_STYLE": "default"}
 
     def __init__(self, *args, **kwargs):
         self._d = self.DEFAULTS.copy()
@@ -157,17 +158,23 @@ class DummyEnv(MutableMapping):
 
 
 def check_exec(input, **kwargs):
-    if not input.endswith('\n'):
-        input += '\n'
+    if not input.endswith("\n"):
+        input += "\n"
     builtins.__xonsh_execer__.exec(input, **kwargs)
     return True
 
 
 def check_eval(input):
-    builtins.__xonsh_env__ = Env({'AUTO_CD': False, 'XONSH_ENCODING': 'utf-8',
-                                  'XONSH_ENCODING_ERRORS': 'strict', 'PATH': []})
+    builtins.__xonsh_env__ = Env(
+        {
+            "AUTO_CD": False,
+            "XONSH_ENCODING": "utf-8",
+            "XONSH_ENCODING_ERRORS": "strict",
+            "PATH": [],
+        }
+    )
     if ON_WINDOWS:
-        builtins.__xonsh_env__['PATHEXT'] = ['.COM', '.EXE', '.BAT', '.CMD']
+        builtins.__xonsh_env__["PATHEXT"] = [".COM", ".EXE", ".BAT", ".CMD"]
     builtins.__xonsh_execer__.eval(input)
     return True
 
@@ -176,6 +183,7 @@ def check_parse(input):
     tree = builtins.__xonsh_execer__.parse(input, ctx=None)
     return tree
 
+
 #
 # Parser tools
 #
@@ -183,15 +191,28 @@ def check_parse(input):
 
 def nodes_equal(x, y):
     __tracebackhide__ = True
-    assert type(x) == type(y), "Ast nodes do not have the same type: '%s' != '%s' " % (type(x), type(y))
+    assert type(x) == type(y), "Ast nodes do not have the same type: '%s' != '%s' " % (
+        type(x),
+        type(y),
+    )
     if isinstance(x, (ast.Expr, ast.FunctionDef, ast.ClassDef)):
-        assert x.lineno == y.lineno, "Ast nodes do not have the same line number : %s != %s" % (x.lineno, y.lineno)
-        assert x.col_offset == y.col_offset, "Ast nodes do not have the same column offset number : %s != %s" % (x.col_offset, y.col_offset)
-    for (xname, xval), (yname, yval) in zip(ast.iter_fields(x),
-                                            ast.iter_fields(y)):
-        assert xname == yname, "Ast nodes fields differ : %s (of type %s) != %s (of type %s)" % (xname, type(xval), yname, type(yval))
-        assert type(xval) == type(yval), "Ast nodes fields differ : %s (of type %s) != %s (of type %s)" % (xname, type(xval), yname, type(yval))
-    for xchild, ychild in zip(ast.iter_child_nodes(x),
-                              ast.iter_child_nodes(y)):
+        assert x.lineno == y.lineno, (
+            "Ast nodes do not have the same line number : %s != %s"
+            % (x.lineno, y.lineno)
+        )
+        assert x.col_offset == y.col_offset, (
+            "Ast nodes do not have the same column offset number : %s != %s"
+            % (x.col_offset, y.col_offset)
+        )
+    for (xname, xval), (yname, yval) in zip(ast.iter_fields(x), ast.iter_fields(y)):
+        assert xname == yname, (
+            "Ast nodes fields differ : %s (of type %s) != %s (of type %s)"
+            % (xname, type(xval), yname, type(yval))
+        )
+        assert type(xval) == type(yval), (
+            "Ast nodes fields differ : %s (of type %s) != %s (of type %s)"
+            % (xname, type(xval), yname, type(yval))
+        )
+    for xchild, ychild in zip(ast.iter_child_nodes(x), ast.iter_child_nodes(y)):
         assert nodes_equal(xchild, ychild), "Ast node children differs"
     return True

xonsh/environ.py

@@ -927,8 +927,8 @@ class Env(cabc.MutableMapping):
         exception = None
         try:
             yield self
-        except Exception as exception:
-            pass
+        except Exception as e:
+            exception = e
         finally:
             # restore the values
             for k, v in old.items():

xonsh/lib/subprocess.xsh

@@ -1,6 +1,8 @@
 """Xonsh extension of the standard library subprocess module, using xonsh for
 subprocess calls"""
-from xonsh.tools import XonshCalledProcessError as CalledProcessError
+from subprocess import CalledProcessError
+
+from xonsh.tools import XonshCalledProcessError
 from xonsh.lib.os import indir