twisted.python.modules.walkModules fails completely if there is an unimportable package anywhere in the path

[exarkun]

exarkun@boson:/tmp/modules$ python
Python 2.5.2 (r252:60911, Jul 31 2008, 17:28:52) 
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from sys import modules
>>> modules['OpenSSL'] = None
>>> from twisted.python.modules import walkModules
>>> list(walkModules())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/exarkun/Projects/Twisted/trunk/twisted/python/modules.py", line 715, in walkModules
    for module in package.walkModules(importPackages=False):
  File "/home/exarkun/Projects/Twisted/trunk/twisted/python/modules.py", line 175, in walkModules
    for package in self.iterModules():
  File "/home/exarkun/Projects/Twisted/trunk/twisted/python/modules.py", line 127, in iterModules
    for placeToLook in self._packagePaths():
  File "/home/exarkun/Projects/Twisted/trunk/twisted/python/modules.py", line 420, in _packagePaths
    load = self.load()
  File "/home/exarkun/Projects/Twisted/trunk/twisted/python/modules.py", line 380, in load
    return self.pathEntry.pythonPath.moduleLoader(self.name)
  File "/home/exarkun/Projects/Twisted/trunk/twisted/python/reflect.py", line 460, in namedAny
    raise ModuleNotFound("No module named %r" % (name,))
twisted.python.reflect.ModuleNotFound: No module named 'OpenSSL'
>>> 

Specifically, this makes the unit test for walkModules rather fragile. In trying to resolve #3111, it is not clear how to make this test pass.

Generally, it's kind of crummy for any one bad module in the path to completely disable this API.

Also, it's kind of crummy that the twisted.python.modules tests load such a huge quantity of Python code, since this makes them rather slow.

[glyph]

Updating the summary because it's actually only un-importable packages which cause problems.

Also, I tried to make a package with a syntactically invalid __init__.py and that didn't trigger the problem; it's something to do with the case which involves the presence of a None in sys.modules.

[glyph]

(In [28722]) Branching to 'walkModulesNone-3419'

[jesstess]

test_modules.py

• needs a copyright bump
• There are some assertEqual in test_nonexistentPaths and test_nonDirectoryPaths that should be assertEquals
• In the docstrings for the two new tests: "If a module has been blacklisted by setting a C{None} key in sys.modules" sounds like it means the same things as "If a package has been explicitly forbidden from importing by setting it to C{None} in sys.modules". If they do mean the same thing, sticking to one description would be clearer.
• test_unimportablePackage says "attempting to walk over it should not load it". Does the existing assertion take care of checking if test_package was loaded, or does there need to be another assert?
• os is imported but unused

modules.py

• needs a copyright bump
• in PythonPath.getitem, the modname parameter needs epytext markup
• in PythonPath.getitem, the @raises statement looks like it's missing a negation.

 builds look good.

[glyph]

(In [28991]) Review feedback for test_modules.

• copyright bump
• assertEqual -> assertEquals
• Fixed docstrings, logic, and names for the two unimportable-package tests to be consistent with each other, and make it clear that they're testing similar functionality of two different APIs.
• added 'is loaded' assertion to test_unimportablePackageWalkModules
• removed unused os import

Refs: #3419

[glyph]

(In [28992]) Review feedback for twisted.python.modules.

• copyright bump
• PythonPath.__getitem__: a better docstring, with
  • @param & @type for modname
  • @raise with a more helpful message (and corrected epytext syntax)
  • separated @return and @rtype

Refs: #3419

[glyph]

 Build results

I think that should be it.

[exarkun]

Needs a news fragment. Otherwise looks good, please add that and merge.

[glyph]

(In [29007]) Merge walkModulesNone-3419

Author: glyph

Reviewer: exarkun, jesstess

Fixes: #3419

twisted.python.modules.walkModules will now handle packages explicitly precluded from importing by a None placed in sys.modules.