Opened 5 years ago
Last modified 5 years ago
#9035 enhancement new
remove trial's support for accepting directory or file names to specify tests to run
Reported by: | Jean-Paul Calderone | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | trial | Keywords: | |
Cc: | Branch: | ||
Author: |
Description
trial currently goes quite a bit out of its way in order to allow tests to be run straight out of a source tree. It used to do even more, adding the working directory to sys.path
to make modules importable without additional efforts on the part of the user.
The *removal* of this latter feature can be (retroactively) described as part of an effort to regularize trial's interaction with Python's module system. Rather than making up a special set of rules for how Python code can be run by trial, trial should instead just accept the normal rules that all other un-sneaky Python code adheres to in this matter.
Not fiddling with sys.path
is one step. Not re-inventing the filename->module mapping will be another good step.
Change History (7)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
I completely agree with this change.
@craig, this wouldn't stop that. It would just mean they pass a fully qualified python name (e.g. incremental
) vs a file path incremental/
. It would not change anything for nearly anybody, and it would remove a lot of ambiguity and potential problems, as trying to figure out what Python module a filepath refers to right now is hilariously bad (I know, because I had to rewrite it for Py3 :) ).
+1, this would remove swathes of edge cases at little cost to users. You should be making packages and modules when doing Python, it is incredibly easy, and the norm. Running your tests from a folder makes no sense in modern Python.
comment:3 Changed 5 years ago by
Craig, do you know of any examples of projects using trial that *can't* use FQPN-based loading?
comment:4 follow-up: 5 Changed 5 years ago by
Here's one use-case which suggests we shouldn't remove this feature:
Sometimes I want to quickly test out a single test-case; I drop the test into a file, test_whatever.py
, and then just do trial test_whatever.py
.
I could do PYTHONPATH="${./:PYTHONPATH}" trial test_whatever
, I suppose, but that's a lot more keystrokes to express a very similar thing. Having to write a setup.py
for a short proof of concept I want to attach to a ticket would be really annoying.
Once you have a full directory full of tests, this use-case falls apart a little; once I have 3 or 4 tests at once then the setup.py
isn't too much overhead. (Sometimes I have a test_a.py
and a test_b.py
and I want to run them both in different orders, but I could probably smash those together into the same file if necessary.)
I'm not sure what the exact behavior of the path-name test running is now, so really addressing this properly might require other changes.
One possible modification; perhaps pathnames should need to have a pathsep
in them somewhere, to eliminate ambiguity with module names?
comment:5 Changed 5 years ago by
Replying to Glyph:
Here's one use-case which suggests we shouldn't remove this feature:
Sometimes I want to quickly test out a single test-case; I drop the test into a file,
test_whatever.py
, and then just dotrial test_whatever.py
.
Okay. This is still a bit more limited than what trial can do now, which is recurse arbitrarily through a directory hierarchy finding all kinds of stuff. So maybe this use-case supports preserving trial <filename>
without any of the directory traversal features?
I could do
PYTHONPATH="${./:PYTHONPATH}" trial test_whatever
, I suppose, but that's a lot more keystrokes to express a very similar thing. Having to write asetup.py
for a short proof of concept I want to attach to a ticket would be really annoying.
You can also, as it turns out, do python -m twisted.trial test_whatever
because python -m twisted.trial
adds the working directory to sys.path
for you!
(twisted-dev) exarkun@baryon:/tmp$ cat test_foo.py print("Hi!") (twisted-dev) exarkun@baryon:/tmp$ trial --version Twisted version: 16.6.0dev0 (twisted-dev) exarkun@baryon:/tmp$ trial test_foo test_foo ... [ERROR] =============================================================================== [ERROR] Traceback (most recent call last): File ".../twisted/src/twisted/trial/runner.py", line 602, in loadByNames things.append(self.findByName(name)) File ".../twisted/src/twisted/trial/runner.py", line 406, in findByName return reflect.namedAny(name) File ".../twisted/src/twisted/python/reflect.py", line 306, in namedAny raise ModuleNotFound("No module named %r" % (name,)) twisted.python.reflect.ModuleNotFound: No module named 'test_foo' test_foo ------------------------------------------------------------------------------- Ran 1 tests in 0.101s FAILED (errors=1) (twisted-dev) exarkun@baryon:/tmp$ python -m twisted.trial test_foo Hi! ------------------------------------------------------------------------------- PASSED (twisted-dev) exarkun@baryon:/tmp$
Does that address your use-case sufficiently?
Once you have a full directory full of tests, this use-case falls apart a little; once I have 3 or 4 tests at once then the
setup.py
isn't too much overhead. (Sometimes I have atest_a.py
and atest_b.py
and I want to run them both in different orders, but I could probably smash those together into the same file if necessary.)I'm not sure what the exact behavior of the path-name test running is now, so really addressing this properly might require other changes.
One possible modification; perhaps pathnames should need to have a
pathsep
in them somewhere, to eliminate ambiguity with module names?
Not a pathsep
(at least, not in the same sense as os.pathsep
). os.sep
, though? Maybe. If non-modules are still supported, getting rid of the ambiguity should be a requirement, I agree.
comment:6 Changed 5 years ago by
@exarkun when someone runs buildbot for the first time, it creates a sample master.cfg that does the following:
- checkout git://github.com/buildbot/pyflakes.git
- runs: trial pyflakes
This example used to work, but with the new behavior in trial, it fails.
comment:7 Changed 5 years ago by
There should of course be a deprecation period before removing the current behavior so as to avoid surprising folks who rely on this feature with breakage and no time to prepare.
No this is not a good change to make at all. There are cases where people use trial to run tests in their own projects.
This is a disruptive change for no good reason and little benefit.
I would like to proceed with https://github.com/twisted/twisted/pull/672 to restore the previous behavior of trial.