Merge pull request #5684 from nicoddemus/errno-nomore

Use OSError subclasses instead of handling errno

Author: asottile

changelog/5684.trivial.rst

@@ -0,0 +1 @@
+Replace manual handling of ``OSError.errno`` in the codebase by new ``OSError`` subclasses (``PermissionError``, ``FileNotFoundError``, etc.).

src/_pytest/assertion/rewrite.py

@@ -116,24 +116,11 @@ def exec_module(self, module):
         write = not sys.dont_write_bytecode
         cache_dir = os.path.join(os.path.dirname(fn), "__pycache__")
         if write:
-            try:
-                os.mkdir(cache_dir)
-            except OSError:
-                e = sys.exc_info()[1].errno
-                if e == errno.EEXIST:
-                    # Either the __pycache__ directory already exists (the
-                    # common case) or it's blocked by a non-dir node. In the
-                    # latter case, we'll ignore it in _write_pyc.
-                    pass
-                elif e in {errno.ENOENT, errno.ENOTDIR}:
-                    # One of the path components was not a directory, likely
-                    # because we're in a zip file.
-                    write = False
-                elif e in {errno.EACCES, errno.EROFS, errno.EPERM}:
-                    state.trace("read only directory: %r" % os.path.dirname(fn))
-                    write = False
-                else:
-                    raise
+            ok = try_mkdir(cache_dir)
+            if not ok:
+                write = False
+                state.trace("read only directory: {}".format(os.path.dirname(fn)))
+
         cache_name = os.path.basename(fn)[:-3] + PYC_TAIL
         pyc = os.path.join(cache_dir, cache_name)
         # Notice that even if we're in a read-only directory, I'm going
@@ -1026,3 +1013,26 @@ def visit_Compare(self, comp):
         else:
             res = load_names[0]
         return res, self.explanation_param(self.pop_format_context(expl_call))
+
+
+def try_mkdir(cache_dir):
+    """Attempts to create the given directory, returns True if successful"""
+    try:
+        os.mkdir(cache_dir)
+    except FileExistsError:
+        # Either the __pycache__ directory already exists (the
+        # common case) or it's blocked by a non-dir node. In the
+        # latter case, we'll ignore it in _write_pyc.
+        return True
+    except (FileNotFoundError, NotADirectoryError):
+        # One of the path components was not a directory, likely
+        # because we're in a zip file.
+        return False
+    except PermissionError:
+        return False
+    except OSError as e:
+        # as of now, EROFS doesn't have an equivalent OSError-subclass
+        if e.errno == errno.EROFS:
+            return False
+        raise
+    return True

src/_pytest/pathlib.py

@@ -1,5 +1,4 @@
 import atexit
-import errno
 import fnmatch
 import itertools
 import operator
@@ -163,14 +162,8 @@ def create_cleanup_lock(p):
     lock_path = get_lock_path(p)
     try:
         fd = os.open(str(lock_path), os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644)
-    except OSError as e:
-        if e.errno == errno.EEXIST:
-            raise EnvironmentError(
-                "cannot create lockfile in {path}".format(path=p)
-            ) from e
-
-        else:
-            raise
+    except FileExistsError as e:
+        raise EnvironmentError("cannot create lockfile in {path}".format(path=p)) from e
     else:
         pid = os.getpid()
         spid = str(pid).encode()

testing/test_assertrewrite.py

@@ -1,4 +1,5 @@
 import ast
+import errno
 import glob
 import importlib
 import os
@@ -7,6 +8,7 @@
 import sys
 import textwrap
 import zipfile
+from functools import partial
 
 import py
 
@@ -1528,3 +1530,43 @@ def test_simple():
 )
 def test_get_assertion_exprs(src, expected):
     assert _get_assertion_exprs(src) == expected
+
+
+def test_try_mkdir(monkeypatch, tmp_path):
+    from _pytest.assertion.rewrite import try_mkdir
+
+    p = tmp_path / "foo"
+
+    # create
+    assert try_mkdir(str(p))
+    assert p.is_dir()
+
+    # already exist
+    assert try_mkdir(str(p))
+
+    # monkeypatch to simulate all error situations
+    def fake_mkdir(p, *, exc):
+        assert isinstance(p, str)
+        raise exc
+
+    monkeypatch.setattr(os, "mkdir", partial(fake_mkdir, exc=FileNotFoundError()))
+    assert not try_mkdir(str(p))
+
+    monkeypatch.setattr(os, "mkdir", partial(fake_mkdir, exc=NotADirectoryError()))
+    assert not try_mkdir(str(p))
+
+    monkeypatch.setattr(os, "mkdir", partial(fake_mkdir, exc=PermissionError()))
+    assert not try_mkdir(str(p))
+
+    err = OSError()
+    err.errno = errno.EROFS
+    monkeypatch.setattr(os, "mkdir", partial(fake_mkdir, exc=err))
+    assert not try_mkdir(str(p))
+
+    # unhandled OSError should raise
+    err = OSError()
+    err.errno = errno.ECHILD
+    monkeypatch.setattr(os, "mkdir", partial(fake_mkdir, exc=err))
+    with pytest.raises(OSError) as exc_info:
+        try_mkdir(str(p))
+    assert exc_info.value.errno == errno.ECHILD