remove trial's support for accepting directory or file names to specify tests to run

[Jean-Paul Calderone]

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.

[Craig Rodrigues]

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.

[hawkowl]

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.

[Jean-Paul Calderone]

Craig, do you know of any examples of projects using trial that *can't* use FQPN-based loading?

[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 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?

[Jean-Paul Calderone]

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 do trial 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 a setup.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 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?

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.

[Craig Rodrigues]

@exarkun when someone runs buildbot for the first time, it creates a sample master.cfg that does the following:

1. checkout ​git://github.com/buildbot/pyflakes.git
2. runs: trial pyflakes

This example used to work, but with the new behavior in trial, it fails.

[Jean-Paul Calderone]

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.