Opened 16 years ago

Closed 15 years ago

#1276 defect closed fixed (fixed)

loadPackage (recurse or otherwise) should respect __path__

Reported by: Jonathan Lange Owned by:
Priority: normal Milestone:
Component: trial Keywords:
Cc: radix, Jonathan Lange, jknight Branch:
Author:

Description


Change History (17)

comment:1 Changed 16 years ago by Jonathan Lange

Implementation in trunk r14850 only handles __path__ for the top-level package.

comment:2 Changed 16 years ago by Jonathan Lange

Do we still want this?

comment:3 Changed 16 years ago by jknight

I still want the repo reorganization, which requires this, yes. I was hoping 
someone else would pick up my initial solution for this issue (which no longer has 
the problem below) and run with it.

comment:4 Changed 16 years ago by Jonathan Lange

Priority: highlow

comment:5 Changed 16 years ago by Jonathan Lange

Some points:

  • It's really hard to do this correctly
  • I don't want to create a new Trial-specific annotation variable for what feels like a Python problem
  • This issue covers a use case which is very rare

comment:6 Changed 16 years ago by jknight

An alternative to the title would be to somehow inform trial of how to reverse map a given filename to a module.

Recap: I want to move twisted into e.g. Core/twisted/internet/ and Project/twisted/project/). Then, make a twisted/__init__.py, with a __path__ that includes both Project/twisted and Core/twisted.

SO, the problem really is: how do I run trial on all the code. Right now, you run "trial twisted". This does more than one might expect: it gets the file for the module "twisted", and does a recursive directory walk over all modules it can find within it. This recursive directory walk has a few problems: it disregards any paths which direct python to look other places for modules, and it only works on modules that are actually in the filesystem (rather than in a zip file).

Anyhow, a recursive directory walk, starting with "." would be okay in the scenario above, except that, after doing the walk, trial then has to make _backwards_ from filename to module. This is not really possible in a general way. So trial just assumes that the file "foo/bar/baz.py" is module "foo.bar.baz". This is often true, but not in the case above, where the file is named "Project/twisted/project/foo.py" and the module it contains is "twisted.project.foo".

So it would also solve this issue to have some way to tell trial "no, map the filenames back to module names like this"

comment:7 Changed 15 years ago by Jonathan Lange

Status: assignednew

comment:8 Changed 15 years ago by jknight

I have a new opinion on how this should work:

  • As before, we should walk everything, importing packages using path.
  • Turn errors when trying to import a package to look for tests into some sort of "skipped" message. Don't bother with any extra custom attributes that might tell it not to look there/etc.
  • Use the python 2.5 function pkgutils.walk_packages to do this, rather than implementing it ourselves. (and backport those functions to t.p.pymodules for use in prior python versions).

comment:9 Changed 15 years ago by jknight

Blaaah, that'll teach me not to use preview. stupid formatting. Reposting readably.

I have a new opinion on how this should work:

  • As before, we should walk everything, importing packages to look for their __path__.
  • Turn errors when trying to import a package to look for tests into some sort of "skipped" message. Don't bother with any extra custom attributes that might tell it not to look there/etc.
  • Use the python 2.5 function pkgutils.walk_packages to do this, rather than implementing it ourselves. (and backport those functions to t.p.pymodules for use in prior python versions).

comment:10 Changed 15 years ago by Jonathan Lange

I like this much more.

Wrt your second point: by "skipped", you mean that import errors should not fail the suite?

comment:11 Changed 15 years ago by jknight

This idea:

ImportErrors, when raised while recursively trawling for packages to look for test modules in, will result in a "skipped" warning of some sort which should be relatively unobtrusive but still noticable. This is because it can be expected that this will happen, but it should still be reported on the off chance (unlikely) that it is preventing a package with tests from being found.

ImportErrors, when raised trying to either run a test, or trying to load a "test_foo" module, will result in an error message as it does right now.

comment:12 Changed 15 years ago by Jonathan Lange

Priority: lownormal

OK. Sounds good (provided it's possible to tell the difference).

I'll look at this after #1940 and #1781 (and its dependants #1936 and #1883) land.

comment:13 Changed 15 years ago by Glyph

The branch for #1940 fixes this as far as I understand it was supposed to be solved, i.e. without actually importing packages, because they might be things that aren't supposed to be imported. It will honor __path__ for already-imported packages, however, and and there is also a flag that can be passed to the one place walkModules is called in trial to import packages as they are discovered, makin the conversion to actually import vs. not be trivial to implement.

comment:14 Changed 15 years ago by Glyph

Resolution: fixed
Status: newclosed

(In [17651]) Make it possible to run trial on tests located within a zip file.

Fixes #1940

Fixes #1276

Author: glyph

Reviewer: jml

This change also introduces a major new piece of functionality, twisted.python.modules, as well as twisted.python.zippath, which allows for FilePath-like access to zipfiles.

As part of changing trial to support zipfiles, another issue was discovered and fixed, which is that trial's sort support was previously broken. This is now fixed, and trial will, by default, sort tests alphabetically now, by fully qualified name.

comment:15 Changed 15 years ago by Jean-Paul Calderone

Resolution: fixed
Status: closedreopened

Reverted merge because of #1952. Re-opening.

comment:16 Changed 15 years ago by Glyph

Resolution: fixed
Status: reopenedclosed

(In [17672]) Re-merge branch for #1940, plus fix for Windows and additional docs.

Fixes #1952 Fixes #1940 Fixes #1276

Author: glyph

Reviewer: jerub

This is a do-over of r17651, hopefully this time without the test failure on the Windows buildslave.

comment:17 Changed 11 years ago by <automation>

Owner: Jonathan Lange deleted
Note: See TracTickets for help on using tickets.