This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5796

py3: conditionalize test-demandimport.py for Python 3
ClosedPublic

Authored by indygreg on Feb 1 2019, 7:48 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG3e89736b98ce: py3: conditionalize test-demandimport.py for Python 3
rHGc15429d02216: py3: conditionalize test-demandimport.py for Python 3
rHGf3c14964dd99: py3: conditionalize test-demandimport.py for Python 3
rHG76600eb1c83c: py3: conditionalize test-demandimport.py for Python 3
Summary

The Python 3 lazy importer uses the LazyLoader that is part of
importlib.

On Python 3 and later, LazyLoader is implemented using a custom module
type that defines a getattribute which triggers module loading.

Furthermore, there are additional differences as well. For example,
it appears that Python 3 will return an existing sys.modules
entry instead of constructing a new module object.

This commit adds additional test coverage for lazy importing
behavior to cover the differences between Python 2 and 3. This
reveals that the test and some lazy import functionality is kinda
busted. For example, the test assumes "contextlib" will be lazy.
But in reality an import before it has already imported contextlib!

There's definitely room to improve the behavior of the demand
importer code, both for Python 2 and 3. But at least the test
passes on Python 3 now.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Feb 1 2019, 7:48 PM
Closed by commit rHG76600eb1c83c: py3: conditionalize test-demandimport.py for Python 3 (authored by indygreg). · Explain WhyFeb 1 2019, 8:33 PM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Feb 1 2019, 10:52 PM

Fixed various check-code errors by black -l 80 -S tests/test-demandimport.py.

indygreg updated this revision to Diff 13697.Feb 2 2019, 12:07 AM
indygreg added a comment.Feb 2 2019, 12:11 AM

I didn't realize this was already queued with formatting changes fixed in flight. Ignore my latest upload.

Revision Contents
Changeset List

PathPackages
Mtests/test-check-code.t (1 line)
Mtests/test-demandimport.py (125 lines)

Diff 13697

tests/test-check-code.t

$ testrepohg locate \ $ testrepohg locate \
> -X contrib/python-zstandard \ > -X contrib/python-zstandard \
> -X hgext/fsmonitor/pywatchman \ > -X hgext/fsmonitor/pywatchman \
> -X mercurial/thirdparty \ > -X mercurial/thirdparty \
> | sed 's-\\-/-g' | "$check_code" --warnings --per-file=0 - || false > | sed 's-\\-/-g' | "$check_code" --warnings --per-file=0 - || false
Skipping i18n/polib.py it has no-che?k-code (glob) Skipping i18n/polib.py it has no-che?k-code (glob)
Skipping mercurial/statprof.py it has no-che?k-code (glob) Skipping mercurial/statprof.py it has no-che?k-code (glob)
Skipping tests/badserverext.py it has no-che?k-code (glob) Skipping tests/badserverext.py it has no-che?k-code (glob)
Skipping tests/test-demandimport.py it has no-che?k-code (glob)
@commands in debugcommands.py should be in alphabetical order. @commands in debugcommands.py should be in alphabetical order.
>>> import re >>> import re
>>> commands = [] >>> commands = []
>>> with open('mercurial/debugcommands.py', 'rb') as fh: >>> with open('mercurial/debugcommands.py', 'rb') as fh:
... for line in fh: ... for line in fh:
... m = re.match(b"^@command\('([a-z]+)", line) ... m = re.match(b"^@command\('([a-z]+)", line)

tests/test-demandimport.py

from __future__ import absolute_import from __future__ import absolute_import, print_function
# no-check-code due to long lines
from mercurial import demandimport from mercurial import demandimport
demandimport.enable() demandimport.enable()
import os import os
import subprocess import subprocess
import sys import sys
import types
# Don't import pycompat because it has too many side-effects.
ispy3 = sys.version_info[0] >= 3
# Only run if demandimport is allowed # Only run if demandimport is allowed
if subprocess.call(['python', '%s/hghave' % os.environ['TESTDIR'], if subprocess.call(['python', '%s/hghave' % os.environ['TESTDIR'],
'demandimport']): 'demandimport']):
sys.exit(80) sys.exit(80)
# We rely on assert, which gets optimized out. # We rely on assert, which gets optimized out.
if sys.flags.optimize: if sys.flags.optimize:
sys.exit(80) sys.exit(80)
if ispy3:
from importlib.util import _LazyModule
try:
from importlib.util import _Module as moduletype
except ImportError:
moduletype = types.ModuleType
else:
moduletype = types.ModuleType
if os.name != 'nt': if os.name != 'nt':
try: try:
import distutils.msvc9compiler import distutils.msvc9compiler
print('distutils.msvc9compiler needs to be an immediate ' print('distutils.msvc9compiler needs to be an immediate '
'importerror on non-windows platforms') 'importerror on non-windows platforms')
distutils.msvc9compiler distutils.msvc9compiler
except ImportError: except ImportError:
pass pass
demandimport.disable() demandimport.disable()
os.environ['HGDEMANDIMPORT'] = 'disable' os.environ['HGDEMANDIMPORT'] = 'disable'
# this enable call should not actually enable demandimport! # this enable call should not actually enable demandimport!
demandimport.enable() demandimport.enable()
from mercurial import node from mercurial import node
# We use assert instead of a unittest test case because having imports inside # We use assert instead of a unittest test case because having imports inside
# functions changes behavior of the demand importer. # functions changes behavior of the demand importer.
if ispy3:
assert not isinstance(node, _LazyModule)
else:
assert f(node) == "<module 'mercurial.node' from '?'>", f(node) assert f(node) == "<module 'mercurial.node' from '?'>", f(node)
# now enable it for real # now enable it for real
del os.environ['HGDEMANDIMPORT'] del os.environ['HGDEMANDIMPORT']
demandimport.enable() demandimport.enable()
# Test access to special attributes through demandmod proxy # Test access to special attributes through demandmod proxy
assert 'mercurial.error' not in sys.modules
from mercurial import error as errorproxy from mercurial import error as errorproxy
if ispy3:
# unsure why this isn't lazy.
assert not isinstance(f, _LazyModule)
assert f(errorproxy) == "<module 'mercurial.error' from '?'>", f(errorproxy)
else:
assert f(errorproxy) == "<unloaded module 'error'>", f(errorproxy) assert f(errorproxy) == "<unloaded module 'error'>", f(errorproxy)
doc = ' '.join(errorproxy.__doc__.split()[:3]) doc = ' '.join(errorproxy.__doc__.split()[:3])
assert doc == 'Mercurial exceptions. This', doc assert doc == 'Mercurial exceptions. This', doc
assert errorproxy.__name__ == 'mercurial.error', errorproxy.__name__ assert errorproxy.__name__ == 'mercurial.error', errorproxy.__name__
# __name__ must be accessible via __dict__ so the relative imports can be # __name__ must be accessible via __dict__ so the relative imports can be
# resolved # resolved
name = errorproxy.__dict__['__name__'] name = errorproxy.__dict__['__name__']
assert name == 'mercurial.error', name assert name == 'mercurial.error', name
if ispy3:
assert not isinstance(errorproxy, _LazyModule)
assert f(errorproxy) == "<module 'mercurial.error' from '?'>", f(errorproxy)
else:
assert f(errorproxy) == "<proxied module 'error'>", f(errorproxy) assert f(errorproxy) == "<proxied module 'error'>", f(errorproxy)
import os import os
if ispy3:
assert not isinstance(os, _LazyModule)
assert f(os) == "<module 'os' from '?'>", f(os)
else:
assert f(os) == "<unloaded module 'os'>", f(os) assert f(os) == "<unloaded module 'os'>", f(os)
assert f(os.system) == '<built-in function system>', f(os.system) assert f(os.system) == '<built-in function system>', f(os.system)
assert f(os) == "<module 'os' from '?'>", f(os) assert f(os) == "<module 'os' from '?'>", f(os)
assert 'mercurial.utils.procutil' not in sys.modules
from mercurial.utils import procutil from mercurial.utils import procutil
if ispy3:
assert isinstance(procutil, _LazyModule)
assert f(procutil) == "<module 'mercurial.utils.procutil' from '?'>", f(procutil)
else:
assert f(procutil) == "<unloaded module 'procutil'>", f(procutil) assert f(procutil) == "<unloaded module 'procutil'>", f(procutil)
assert f(procutil.system) == '<function system at 0x?>', f(procutil.system) assert f(procutil.system) == '<function system at 0x?>', f(procutil.system)
assert procutil.__class__ == moduletype, procutil.__class__
assert f(procutil) == "<module 'mercurial.utils.procutil' from '?'>", f(procutil) assert f(procutil) == "<module 'mercurial.utils.procutil' from '?'>", f(procutil)
assert f(procutil.system) == '<function system at 0x?>', f(procutil.system) assert f(procutil.system) == '<function system at 0x?>', f(procutil.system)
assert 'mercurial.hgweb' not in sys.modules
from mercurial import hgweb from mercurial import hgweb
if ispy3:
assert not isinstance(hgweb, _LazyModule)
assert f(hgweb) == "<module 'mercurial.hgweb' from '?'>", f(hgweb)
assert isinstance(hgweb.hgweb_mod, _LazyModule)
assert f(hgweb.hgweb_mod) == "<module 'mercurial.hgweb.hgweb_mod' from '?'>", f(hgweb.hgweb_mod)
else:
assert f(hgweb) == "<unloaded module 'hgweb'>", f(hgweb) assert f(hgweb) == "<unloaded module 'hgweb'>", f(hgweb)
assert f(hgweb.hgweb_mod) == "<unloaded module 'hgweb_mod'>", f(hgweb.hgweb_mod) assert f(hgweb.hgweb_mod) == "<unloaded module 'hgweb_mod'>", f(hgweb.hgweb_mod)
assert f(hgweb) == "<module 'mercurial.hgweb' from '?'>", f(hgweb) assert f(hgweb) == "<module 'mercurial.hgweb' from '?'>", f(hgweb)
import re as fred import re as fred
if ispy3:
assert not isinstance(fred, _LazyModule)
assert f(fred) == "<module 're' from '?'>"
else:
assert f(fred) == "<unloaded module 're'>", f(fred) assert f(fred) == "<unloaded module 're'>", f(fred)
import re as remod import re as remod
if ispy3:
assert not isinstance(remod, _LazyModule)
assert f(remod) == "<module 're' from '?'>"
else:
assert f(remod) == "<unloaded module 're'>", f(remod) assert f(remod) == "<unloaded module 're'>", f(remod)
import sys as re import sys as re
if ispy3:
assert not isinstance(re, _LazyModule)
assert f(re) == "<module 'sys' (built-in)>"
else:
assert f(re) == "<unloaded module 'sys'>", f(re) assert f(re) == "<unloaded module 'sys'>", f(re)
if ispy3:
assert not isinstance(fred, _LazyModule)
assert f(fred) == "<module 're' from '?'>", f(fred)
else:
assert f(fred) == "<unloaded module 're'>", f(fred) assert f(fred) == "<unloaded module 're'>", f(fred)
assert f(fred.sub) == '<function sub at 0x?>', f(fred.sub) assert f(fred.sub) == '<function sub at 0x?>', f(fred.sub)
if ispy3:
assert not isinstance(fred, _LazyModule)
assert f(fred) == "<module 're' from '?'>", f(fred)
else:
assert f(fred) == "<proxied module 're'>", f(fred) assert f(fred) == "<proxied module 're'>", f(fred)
remod.escape # use remod remod.escape # use remod
assert f(remod) == "<module 're' from '?'>", f(remod) assert f(remod) == "<module 're' from '?'>", f(remod)
if ispy3:
assert not isinstance(re, _LazyModule)
assert f(re) == "<module 'sys' (built-in)>"
assert f(type(re.stderr)) == "<class '_io.TextIOWrapper'>", f(type(re.stderr))
assert f(re) == "<module 'sys' (built-in)>"
else:
assert f(re) == "<unloaded module 'sys'>", f(re) assert f(re) == "<unloaded module 'sys'>", f(re)
assert f(re.stderr) == "<open file '<whatever>', mode 'w' at 0x?>", f(re.stderr) assert f(re.stderr) == "<open file '<whatever>', mode 'w' at 0x?>", f(re.stderr)
assert f(re) == "<proxied module 'sys'>", f(re) assert f(re) == "<proxied module 'sys'>", f(re)
import contextlib import contextlib
if ispy3:
assert not isinstance(contextlib, _LazyModule)
assert f(contextlib) == "<module 'contextlib' from '?'>"
else:
assert f(contextlib) == "<unloaded module 'contextlib'>", f(contextlib) assert f(contextlib) == "<unloaded module 'contextlib'>", f(contextlib)
try: try:
from contextlib import unknownattr from contextlib import unknownattr
assert False, ('no demandmod should be created for attribute of non-package ' assert False, ('no demandmod should be created for attribute of non-package '
'module:\ncontextlib.unknownattr = %s' % f(unknownattr)) 'module:\ncontextlib.unknownattr = %s' % f(unknownattr))
except ImportError as inst: except ImportError as inst:
assert rsub(r"'", '', str(inst)) == 'cannot import name unknownattr' assert rsub(r"'", '', str(inst)).startswith('cannot import name unknownattr')
from mercurial import util from mercurial import util
# Unlike the import statement, __import__() function should not raise # Unlike the import statement, __import__() function should not raise
# ImportError even if fromlist has an unknown item # ImportError even if fromlist has an unknown item
# (see Python/import.c:import_module_level() and ensure_fromlist()) # (see Python/import.c:import_module_level() and ensure_fromlist())
contextlibimp = __import__('contextlib', globals(), locals(), ['unknownattr']) contextlibimp = __import__('contextlib', globals(), locals(), ['unknownattr'])
assert f(contextlibimp) == "<module 'contextlib' from '?'>", f(contextlibimp) assert f(contextlibimp) == "<module 'contextlib' from '?'>", f(contextlibimp)
assert not util.safehasattr(contextlibimp, 'unknownattr') assert not util.safehasattr(contextlibimp, 'unknownattr')