bpo-33861: Minor improvements of tests for os.path. (GH-7715)

* Test exists(), lexists(), isdir(), isfile(), islink(), ismount()
  with bytes paths.
* Remove unneeded silencing DeprecationWarning for ismount() with
  bytes path.
* Test common functions with unencodable and undecodable paths.
* Minor clean up and refactoring.

2 files changed, 61 insertions, 47 deletions

diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py
index 9ed5390..8b291cd 100644
--- a/Lib/test/test_genericpath.py
+++ b/Lib/test/test_genericpath.py
@@ -127,17 +127,20 @@ class GenericTest:
 
     def test_exists(self):
         filename = support.TESTFN
+        bfilename = os.fsencode(filename)
         self.addCleanup(support.unlink, filename)
 
         self.assertIs(self.pathmodule.exists(filename), False)
+        self.assertIs(self.pathmodule.exists(bfilename), False)
 
-        with open(filename, "xb") as f:
-            f.write(b"foo")
+        create_file(filename)
 
         self.assertIs(self.pathmodule.exists(filename), True)
+        self.assertIs(self.pathmodule.exists(bfilename), True)
 
-        if not self.pathmodule == genericpath:
+        if self.pathmodule is not genericpath:
             self.assertIs(self.pathmodule.lexists(filename), True)
+            self.assertIs(self.pathmodule.lexists(bfilename), True)
 
     @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()")
     def test_exists_fd(self):
@@ -149,37 +152,45 @@ class GenericTest:
             os.close(w)
         self.assertFalse(self.pathmodule.exists(r))
 
-    def test_isdir_file(self):
+    def test_isdir(self):
         filename = support.TESTFN
-        self.addCleanup(support.unlink, filename)
-        self.assertIs(self.pathmodule.isdir(filename), False)
-
-        create_file(filename)
+        bfilename = os.fsencode(filename)
         self.assertIs(self.pathmodule.isdir(filename), False)
+        self.assertIs(self.pathmodule.isdir(bfilename), False)
 
-    def test_isdir_dir(self):
-        filename = support.TESTFN
-        self.addCleanup(support.rmdir, filename)
-        self.assertIs(self.pathmodule.isdir(filename), False)
+        try:
+            create_file(filename)
+            self.assertIs(self.pathmodule.isdir(filename), False)
+            self.assertIs(self.pathmodule.isdir(bfilename), False)
+        finally:
+            support.unlink(filename)
 
-        os.mkdir(filename)
-        self.assertIs(self.pathmodule.isdir(filename), True)
+        try:
+            os.mkdir(filename)
+            self.assertIs(self.pathmodule.isdir(filename), True)
+            self.assertIs(self.pathmodule.isdir(bfilename), True)
+        finally:
+            support.rmdir(filename)
 
-    def test_isfile_file(self):
+    def test_isfile(self):
         filename = support.TESTFN
-        self.addCleanup(support.unlink, filename)
+        bfilename = os.fsencode(filename)
         self.assertIs(self.pathmodule.isfile(filename), False)
+        self.assertIs(self.pathmodule.isfile(bfilename), False)
 
-        create_file(filename)
-        self.assertIs(self.pathmodule.isfile(filename), True)
-
-    def test_isfile_dir(self):
-        filename = support.TESTFN
-        self.addCleanup(support.rmdir, filename)
-        self.assertIs(self.pathmodule.isfile(filename), False)
+        try:
+            create_file(filename)
+            self.assertIs(self.pathmodule.isfile(filename), True)
+            self.assertIs(self.pathmodule.isfile(bfilename), True)
+        finally:
+            support.unlink(filename)
 
-        os.mkdir(filename)
-        self.assertIs(self.pathmodule.isfile(filename), False)
+        try:
+            os.mkdir(filename)
+            self.assertIs(self.pathmodule.isfile(filename), False)
+            self.assertIs(self.pathmodule.isfile(bfilename), False)
+        finally:
+            support.rmdir(filename)
 
     def test_samefile(self):
         file1 = support.TESTFN
@@ -280,15 +291,25 @@ class TestGenericTest(GenericTest, unittest.TestCase):
     # and is only meant to be inherited by others.
     pathmodule = genericpath
 
-    def test_null_bytes(self):
+    def test_invalid_paths(self):
         for attr in GenericTest.common_attributes:
             # os.path.commonprefix doesn't raise ValueError
             if attr == 'commonprefix':
                 continue
+            func = getattr(self.pathmodule, attr)
             with self.subTest(attr=attr):
-                with self.assertRaises(ValueError) as cm:
-                    getattr(self.pathmodule, attr)('/tmp\x00abcds')
-                self.assertIn('embedded null', str(cm.exception))
+                try:
+                    func('/tmp\udfffabcds')
+                except (OSError, UnicodeEncodeError):
+                    pass
+                try:
+                    func(b'/tmp\xffabcds')
+                except (OSError, UnicodeDecodeError):
+                    pass
+                with self.assertRaisesRegex(ValueError, 'embedded null'):
+                    func('/tmp\x00abcds')
+                with self.assertRaisesRegex(ValueError, 'embedded null'):
+                    func(b'/tmp\x00abcds')
 
 # Following TestCase is not supposed to be run from test_genericpath.
 # It is inherited by other test modules (macpath, ntpath, posixpath).
@@ -529,5 +550,5 @@ class PathLikeTests(unittest.TestCase):
         self.assertTrue(os.path.samefile(self.file_path, self.file_name))
 
 
-if __name__=="__main__":
+if __name__ == "__main__":
     unittest.main()
diff --git a/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py
index 96b267c..9476ede 100644
--- a/Lib/test/test_posixpath.py
+++ b/Lib/test/test_posixpath.py
@@ -153,27 +153,20 @@ class PosixPathTest(unittest.TestCase):
     def test_islink(self):
         self.assertIs(posixpath.islink(support.TESTFN + "1"), False)
         self.assertIs(posixpath.lexists(support.TESTFN + "2"), False)
-        f = open(support.TESTFN + "1", "wb")
-        try:
+        with open(support.TESTFN + "1", "wb") as f:
             f.write(b"foo")
-            f.close()
-            self.assertIs(posixpath.islink(support.TESTFN + "1"), False)
-            if support.can_symlink():
-                os.symlink(support.TESTFN + "1", support.TESTFN + "2")
-                self.assertIs(posixpath.islink(support.TESTFN + "2"), True)
-                os.remove(support.TESTFN + "1")
-                self.assertIs(posixpath.islink(support.TESTFN + "2"), True)
-                self.assertIs(posixpath.exists(support.TESTFN + "2"), False)
-                self.assertIs(posixpath.lexists(support.TESTFN + "2"), True)
-        finally:
-            if not f.close():
-                f.close()
+        self.assertIs(posixpath.islink(support.TESTFN + "1"), False)
+        if support.can_symlink():
+            os.symlink(support.TESTFN + "1", support.TESTFN + "2")
+            self.assertIs(posixpath.islink(support.TESTFN + "2"), True)
+            os.remove(support.TESTFN + "1")
+            self.assertIs(posixpath.islink(support.TESTFN + "2"), True)
+            self.assertIs(posixpath.exists(support.TESTFN + "2"), False)
+            self.assertIs(posixpath.lexists(support.TESTFN + "2"), True)
 
     def test_ismount(self):
         self.assertIs(posixpath.ismount("/"), True)
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore", DeprecationWarning)
-            self.assertIs(posixpath.ismount(b"/"), True)
+        self.assertIs(posixpath.ismount(b"/"), True)
 
     def test_ismount_non_existent(self):
         # Non-existent mountpoint.