Skip to content

Commit

Permalink
bpo-1812: Fix newline conversion when doctest.testfile loads from a p…
Browse files Browse the repository at this point in the history
…ackage 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 e0b8101)

Co-authored-by: Peter Donis <peterdonis@alum.mit.edu>
  • Loading branch information
miss-islington and pdonis committed Mar 26, 2020
1 parent ea0eeb8 commit 9387678
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 5 deletions.
9 changes: 8 additions & 1 deletion Lib/doctest.py
Expand Up @@ -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)
Expand All @@ -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

Expand Down
93 changes: 89 additions & 4 deletions Lib/test/test_doctest.py
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)


@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:
Expand All @@ -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"""
Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
Expand Up @@ -402,6 +402,7 @@ Walter Dörwald
Jaromir Dolecek
Zsolt Dollenstein
Brendan Donegan
Peter Donis
Ismail Donmez
Ray Donnelly
Robert Donohue
Expand Down
@@ -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.

0 comments on commit 9387678

Please sign in to comment.