liqd / adhocracy

Adhocracy is a policy drafting and decision making software for distributed groups and open institutions.
GNU Affero General Public License v3.0
150 stars 37 forks source link

test fails during import #180

Open phihag opened 11 years ago

phihag commented 11 years ago

Currently, the test fails during import:

$ bin/test -x
E
======================================================================
ERROR: Failure: AttributeError ('module' object has no attribute 'model')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/phihag/adhocracy_buildout/lib/python2.6/site-packages/nose-1.2.1-py2.6.egg/nose/loader.py", line 390, in loadTestsFromName
    addr.filename, addr.module)
  File "/home/phihag/adhocracy_buildout/lib/python2.6/site-packages/nose-1.2.1-py2.6.egg/nose/importer.py", line 39, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/home/phihag/adhocracy_buildout/lib/python2.6/site-packages/nose-1.2.1-py2.6.egg/nose/importer.py", line 84, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/phihag/adhocracy_buildout/src/adhocracy/adhocracy/lib/cache/__init__.py", line 3, in <module>
    import adhocracy.model as model
AttributeError: 'module' object has no attribute 'model'
-------------------- >> begin captured logging << --------------------
pylons.configuration: DEBUG: Initializing configuration, package: 'adhocracy'
adhocracy.lib.app_globals: INFO: Memcache set up
adhocracy.lib.app_globals: DEBUG: Flushing cache
routes.middleware: DEBUG: Initialized with method overriding = True, and path info altering = True
adhocracy.lib.instance.discriminator: DEBUG: Host name: test.lan.
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 1 test in 0.788s

FAILED (errors=1)

The error message is rather curious - it seems to fail during a time when adhocracy.model is already imported, but model not assigned to adhocracy.

phihag commented 11 years ago

This seems to be related to an improper generation of bin/tests, which uses physical paths for the adhocracy libraries in sys.path, and virtual ones for the test directory.

nidico commented 11 years ago

Doesn't occur to me, all tests pass in my case.

I had 2 erroneous tests with ImportError (cssselect seems not to be installed. See http://packages.python.org/cssselect/), however these are fixed with your recent commit 2d326af5c046ab514b86eaf7b7d86ad6977809c9.

phihag commented 11 years ago

That's most likely because in your case, physical and virtual paths are identical. Part of the blame belongs of course to nosetests, which uses its own importer instead of letting Python worry about it.

phihag commented 11 years ago

Ok, I tracked it down, and the ultimate cause (aside from understandable nosetests limitations) is buildout, which does not properly handle virtual paths and comes with very notations of what paths are (most likely coming coming from DOS/Windows).

Since these are very much ingrained into a lot of the source (for example, they calculate the common ancestor of paths for no apparent reason, instead of using simply using relative paths), the patch for it will be nontrivial. On the upside, the patch will also fix a swatch of unnecesarily absolute paths which prevent moving adhocracy installations around at the moment.

For now, to not run in this immediate issue, it's sufficient to make virtual paths identical to physical ones or edit bin/test to make all references to the adhocracy directory use the same virtual path.

nidico commented 11 years ago

What's a virtual path? I only know of absolute vs relative paths.

phihag commented 11 years ago

As used here, a virtual path is the path specified by the user, as opposed to the physical path (or more precisely one of the physical paths), which could be regarded as the "proper" filename (the result of readlink -f).

The two diverge in the presence of mountpoints which don't simply map block devices to filesystems, symlinks, and (multiple) hardlinks.

nidico commented 11 years ago

Understand... I never use multiple hardlinks to the same file actively (apart from using them through other tools), but there may be reasons to do so... Feel free to file a ticket / pull request against buildout, I guess they're simply not aware of that issue.

phihag commented 11 years ago

Multiple hardlinks to the same file are indeed pretty rare, but symlinks and mountpoints are actually part of our informal installation setup here. That allows us to delete the whole adhocracy directory with impunity, which can be handy to test the setup or in the case of inadvertently installing the wrong dependencies.

nidico commented 11 years ago

I haven't had any issues with symlinks, as they seem to be resolved to their canonical physical paths by buildout. With "mountpoints which don't simply map block devices to filesystems", you mean bind mounts? What's the most simple setup (without multiple hardlinks to the same file), which produces this problem?

Have you tried the buildout option relative-paths=true?

phihag commented 11 years ago

Not only bind mounts, we also use UnionFS, VirtualBox, sshfs mounts. The most simple setup is symlinking adhocracy_buildout/src/adhocracy to somewhere else (like ~/adhocracy.host) and then re-executing buildout. The resulting bin/test then looks like

sys.path[0:0] = [
    '/home/phihag/adhocracy.host',
    '/home/phihag/adhocracy_buildout/eggs/BeautifulSoup-3.2.1-py2.6.egg',
    # ...
    ]

import nose

if __name__ == '__main__':
    sys.exit(nose.main(argv=['nose', '-s', '-q',
      '--with-pylons=/home/phihag/adhocracy_buildout/src/adhocracy/test.ini',
      '/home/phihag/adhocracy_buildout/src/adhocracy/']+sys.argv[1:]))

Despite the name and frequently cited value of False, relative_paths is - at least in our buildout - not a boolean value, but a byte? string (the common paths of all scripts).

The buildout file that deal with this - mainly zc/buildout/easy_install.py around line 1330 - is peppered with needless invocations to os.path.abspath, os.path.normpath and friends. However, I don't think there is a simple small patch to fix the problem (not to speak that this file could use an overhaul in general - there are plenty of methods with 10+ arguments in there).

nidico commented 11 years ago

relative-path=true (dash, not underscore) is an option which can be used in a [buildout] section of a buildout config file. This is then used to calculate the relative_paths parameter (a string representing the base path), which is then further used internally...

Using this option indeed changes some generated paths to being relative paths (e.g. in bin/buildout after running bootstrap), but unfortunately doesn't resolve your problem, as the problem is not about relative vs absolute, and the generated paths in bin/test are still physical, not virtual.

Understood the actual problem now :)