[Python-checkins] bpo-1812: Fix newline conversion when doctest.testfile loads from a package whose loader has a get_data method (GH-17385)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/9387678f8a580726aca5f836b2c50456f236ecbb
commit: 9387678f8a580726aca5f836b2c50456f236ecbb
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2020-03-26T09:18:55-07:00
summary:

bpo-1812: Fix newline conversion when doctest.testfile loads from a package whose loader has a get_data method (GH-17385)


This pull request fixes the newline conversion bug originally reported in bpo-1812. When that issue was originally submitted, the open builtin did not default to universal newline mode; now it does, which makes the issue fix simpler, since the only code path that needs to be changed is the one in doctest._load_testfile where the file is loaded from a package whose loader has a get_data method.
(cherry picked from commit e0b8101492f6c61dee831425b4d3dae39a953599)

Co-authored-by: Peter Donis <peterdonis at alum.mit.edu>

files:
A Misc/NEWS.d/next/Tests/2019-11-25-21-46-47.bpo-1812.sAbTbY.rst
M Lib/doctest.py
M Lib/test/test_doctest.py
M Misc/ACKS

diff --git a/Lib/doctest.py b/Lib/doctest.py
index 2a4b8c7dae012..47917b486e000 100644
--- a/Lib/doctest.py
+++ b/Lib/doctest.py
@@ -211,6 +211,13 @@ def _normalize_module(module, depth=2):
     else:
         raise TypeError("Expected a module, string, or None")
 
+def _newline_convert(data):
+    # We have two cases to cover and we need to make sure we do
+    # them in the right order
+    for newline in ('\r\n', '\r'):
+        data = data.replace(newline, '\n')
+    return data
+
 def _load_testfile(filename, package, module_relative, encoding):
     if module_relative:
         package = _normalize_module(package, 3)
@@ -221,7 +228,7 @@ def _load_testfile(filename, package, module_relative, encoding):
                 file_contents = file_contents.decode(encoding)
                 # get_data() opens files as 'rb', so one must do the equivalent
                 # conversion as universal newlines would do.
-                return file_contents.replace(os.linesep, '\n'), filename
+                return _newline_convert(file_contents), filename
     with open(filename, encoding=encoding) as f:
         return f.read(), filename
 
diff --git a/Lib/test/test_doctest.py b/Lib/test/test_doctest.py
index 5f891c50d8fca..c2f0568b5567c 100644
--- a/Lib/test/test_doctest.py
+++ b/Lib/test/test_doctest.py
@@ -8,8 +8,12 @@
 import os
 import sys
 import importlib
+import importlib.abc
+import importlib.util
 import unittest
 import tempfile
+import shutil
+import contextlib
 
 # NOTE: There are some additional tests relating to interaction with
 #       zipimport in the test_zipimport_support test module.
@@ -437,7 +441,7 @@ def basics(): r"""
     >>> tests = finder.find(sample_func)
 
     >>> print(tests)  # doctest: +ELLIPSIS
-    [<DocTest sample_func from ...:21 (1 example)>]
+    [<DocTest sample_func from ...:25 (1 example)>]
 
 The exact name depends on how test_doctest was invoked, so allow for
 leading path components.
@@ -2659,12 +2663,52 @@ def test_testfile(): r"""
     >>> sys.argv = save_argv
 """
 
+class TestImporter(importlib.abc.MetaPathFinder, importlib.abc.ResourceLoader):
+
+    def find_spec(self, fullname, path, target=None):
+        return importlib.util.spec_from_file_location(fullname, path, loader=self)
+
+    def get_data(self, path):
+        with open(path, mode='rb') as f:
+            return f.read()
+
+class TestHook:
+
+    def __init__(self, pathdir):
+        self.sys_path = sys.path[:]
+        self.meta_path = sys.meta_path[:]
+        self.path_hooks = sys.path_hooks[:]
+        sys.path.append(pathdir)
+        sys.path_importer_cache.clear()
+        self.modules_before = sys.modules.copy()
+        self.importer = TestImporter()
+        sys.meta_path.append(self.importer)
+
+    def remove(self):
+        sys.path[:] = self.sys_path
+        sys.meta_path[:] = self.meta_path
+        sys.path_hooks[:] = self.path_hooks
+        sys.path_importer_cache.clear()
+        sys.modules.clear()
+        sys.modules.update(self.modules_before)
+
+
+ at contextlib.contextmanager
+def test_hook(pathdir):
+    hook = TestHook(pathdir)
+    try:
+        yield hook
+    finally:
+        hook.remove()
+
+
 def test_lineendings(): r"""
-*nix systems use \n line endings, while Windows systems use \r\n.  Python
+*nix systems use \n line endings, while Windows systems use \r\n, and
+old Mac systems used \r, which Python still recognizes as a line ending.  Python
 handles this using universal newline mode for reading files.  Let's make
 sure doctest does so (issue 8473) by creating temporary test files using each
-of the two line disciplines.  One of the two will be the "wrong" one for the
-platform the test is run on.
+of the three line disciplines.  At least one will not match either the universal
+newline \n or os.linesep for the platform the test is run on.
 
 Windows line endings first:
 
@@ -2687,6 +2731,47 @@ def test_lineendings(): r"""
     TestResults(failed=0, attempted=1)
     >>> os.remove(fn)
 
+And finally old Mac line endings:
+
+    >>> fn = tempfile.mktemp()
+    >>> with open(fn, 'wb') as f:
+    ...     f.write(b'Test:\r\r  >>> x = 1 + 1\r\rDone.\r')
+    30
+    >>> doctest.testfile(fn, module_relative=False, verbose=False)
+    TestResults(failed=0, attempted=1)
+    >>> os.remove(fn)
+
+Now we test with a package loader that has a get_data method, since that
+bypasses the standard universal newline handling so doctest has to do the
+newline conversion itself; let's make sure it does so correctly (issue 1812).
+We'll write a file inside the package that has all three kinds of line endings
+in it, and use a package hook to install a custom loader; on any platform,
+at least one of the line endings will raise a ValueError for inconsistent
+whitespace if doctest does not correctly do the newline conversion.
+
+    >>> dn = tempfile.mkdtemp()
+    >>> pkg = os.path.join(dn, "doctest_testpkg")
+    >>> os.mkdir(pkg)
+    >>> support.create_empty_file(os.path.join(pkg, "__init__.py"))
+    >>> fn = os.path.join(pkg, "doctest_testfile.txt")
+    >>> with open(fn, 'wb') as f:
+    ...     f.write(
+    ...         b'Test:\r\n\r\n'
+    ...         b'  >>> x = 1 + 1\r\n\r\n'
+    ...         b'Done.\r\n'
+    ...         b'Test:\n\n'
+    ...         b'  >>> x = 1 + 1\n\n'
+    ...         b'Done.\n'
+    ...         b'Test:\r\r'
+    ...         b'  >>> x = 1 + 1\r\r'
+    ...         b'Done.\r'
+    ...     )
+    95
+    >>> with test_hook(dn):
+    ...     doctest.testfile("doctest_testfile.txt", package="doctest_testpkg", verbose=False)
+    TestResults(failed=0, attempted=3)
+    >>> shutil.rmtree(dn)
+
 """
 
 def test_testmod(): r"""
diff --git a/Misc/ACKS b/Misc/ACKS
index 684d18ac1c558..27ef39754afd9 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -402,6 +402,7 @@ Walter Dörwald
 Jaromir Dolecek
 Zsolt Dollenstein
 Brendan Donegan
+Peter Donis
 Ismail Donmez
 Ray Donnelly
 Robert Donohue
diff --git a/Misc/NEWS.d/next/Tests/2019-11-25-21-46-47.bpo-1812.sAbTbY.rst b/Misc/NEWS.d/next/Tests/2019-11-25-21-46-47.bpo-1812.sAbTbY.rst
new file mode 100644
index 0000000000000..7ffe90d55a4e7
--- /dev/null
+++ b/Misc/NEWS.d/next/Tests/2019-11-25-21-46-47.bpo-1812.sAbTbY.rst
@@ -0,0 +1,2 @@
+Fix newline handling in doctest.testfile when loading from a package whose
+loader has a get_data method. Patch by Peter Donis.