Move tests out of package to top level directory

[jdufresne]

Cleanly separates tests from the package itself. Prevents the tests being installed to site-packages. Tests are still distributed with the source distribution by MANIFEST.in.

Avoids installing tests in production environments, leading to less total code in the environment.

[giampaolo]

There is a reason why I included tests in psutil namespace and source distribution: the ability to test psutil installation with "python -m psutil.tests" without having to git clone the repo.

[jdufresne]

Hmm, I understand.

FYI, the tests are also included in the source distribution, so a git clone isn't necessary if the package is already downloaded.

As a user of the package, I consider it unwanted pollution of my site-packages directory. For a tightly controlled production environment, I try to keep runnable code to a minimum to reduce the surface area of potential bugs (or worse, exploitable code). Having a bunch of code that is not intended to run is unwanted for these scenarios.

The tests are for development of the package, not for using it. Should I want to test the psutil package or develop it, I can continue to use the source distribution or a git clone of the project. But the tests aren't necessary for my installed production environment.

Most of the packages I interact with do not install their tests to site-packages. psutil is one of very few. So it seems the community has settled on a convention to not install tests.

Ultimately, up to you. Just making a suggestion as a user. Thanks for the great package!

[jdufresne]

I came across the following articles which I thought you might find interesting. They outline why a project may prefer a "tests outside application code" approach.

• https://docs.pytest.org/en/latest/goodpractices.html#tests-outside-application-code
• https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure

The end of the 2nd article has the summary:

You'll notice that I don't include the tests in the installed packages. Because:

Module discovery tools will trip over your test modules. Strange things usually happen in test module. The help builtin does module discovery. E.g.:

   >>> help('modules')
   Please wait a moment while I gather a list of all available modules...

   __future__          antigravity         html                select
   ...

Tests usually require additional dependencies to run, so they aren't useful by their own - you can't run them directly.

Tests are concerned with development, not usage.

It's extremely unlikely that the user of the library will run the tests instead of the library's developer. E.g.: you don't run the tests for Django while testing your apps - Django is already tested.

[jdufresne]

I rebased to resolve merge conflicts.

[giampaolo]

OK, you convinced me. Mainly I think removing tests from site-packages dir is the big win here so I welcome this change. I added a couple of (minor) inline comments, then it's good to go.

[giampaolo]

Can you please run make generate-manifest instead?

[giampaolo]

This whole part ("there are 2 ways of running tests") should go away as from now on there is only one.

[giampaolo]

nice, this is the main enhancement (remove tests from site-packages)

[giampaolo]

Also here, the "there are 2 ways of running tests" part should go away

[jdufresne]

Thanks for the review. I've made the suggested changes.

For the changed documentation, if you like me to change any of the wording, please let me know. It is no problem to.

[giampaolo]

This is outdated. Let's just remove "this is invoked by...."

[jdufresne]

Done!

[giampaolo]

Excellent work. Merged.
And BTW, I'm usually alone when making these kind of "cosmetic" / "perfectionist" changes. I'm glad there's somebody else out there. =)

[giampaolo]

There's a problem with Python 2.7:

$ make test
...
PYTHONWARNINGS=all PSUTIL_TESTING=1 PSUTIL_DEBUG=1 python tests/__main__.py
Traceback (most recent call last):
  File "tests/__main__.py", line 22, in <module>
    from tests import PYTHON_EXE
ImportError: cannot import name PYTHON_EXE
Makefile:111: recipe for target 'test' failed

This:

(Pdb) import tests
(Pdb) tests
<module 'tests' from '/home/giampaolo/.local/lib/python2.7/site-packages/tests/__init__.pyc'>

[jdufresne]

I'm not seeing that error. When I run make tests or python tests/__main__.py all tests run. Do you have additional information on how to trigger the error?

[jdufresne]

<module 'tests' from '/home/giampaolo/.local/lib/python2.7/site-packages/tests/init.pyc'>

Why do you have a module named tests in your site-packages? What is that exactly?

[giampaolo]

Looks like it's some third party package's tests:

ls -l  /home/giampaolo/.local/lib/python2.7/site-packages/tests/
total 480K
drwxrwxr-x 4 giampaolo giampaolo 4,0K 2016-10-25 16:56 contrib/
-rw-rw-r-- 1 giampaolo giampaolo 1,1K 2016-10-25 16:56 conftest.py
-rw-rw-r-- 1 giampaolo giampaolo  914 2016-10-25 16:56 conftest.pyc
-rw-rw-r-- 1 giampaolo giampaolo 4,0K 2016-10-25 16:56 http_mock.py
-rw-rw-r-- 1 giampaolo giampaolo 4,4K 2016-10-25 16:56 http_mock.pyc
-rw-rw-r-- 1 giampaolo giampaolo    0 2016-10-25 16:56 __init__.py
-rw-rw-r-- 1 giampaolo giampaolo  139 2016-10-25 16:56 __init__.pyc
-rw-rw-r-- 1 giampaolo giampaolo 102K 2016-10-25 16:56 test_client.py
-rw-rw-r-- 1 giampaolo giampaolo  94K 2016-10-25 16:56 test_client.pyc
-rw-rw-r-- 1 giampaolo giampaolo  11K 2016-10-25 16:56 test_clientsecrets.py
-rw-rw-r-- 1 giampaolo giampaolo  11K 2016-10-25 16:56 test_clientsecrets.pyc
-rw-rw-r-- 1 giampaolo giampaolo  12K 2016-10-25 16:56 test_crypt.py
-rw-rw-r-- 1 giampaolo giampaolo  12K 2016-10-25 16:56 test_crypt.pyc
-rw-rw-r-- 1 giampaolo giampaolo 9,5K 2016-10-25 16:56 test_file.py
-rw-rw-r-- 1 giampaolo giampaolo 8,6K 2016-10-25 16:56 test_file.pyc
-rw-rw-r-- 1 giampaolo giampaolo 8,9K 2016-10-25 16:56 test__helpers.py
-rw-rw-r-- 1 giampaolo giampaolo  13K 2016-10-25 16:56 test__helpers.pyc
-rw-rw-r-- 1 giampaolo giampaolo  13K 2016-10-25 16:56 test_jwt.py
-rw-rw-r-- 1 giampaolo giampaolo  14K 2016-10-25 16:56 test_jwt.pyc
-rw-rw-r-- 1 giampaolo giampaolo 2,0K 2016-10-25 16:56 test__pkce.py
-rw-rw-r-- 1 giampaolo giampaolo 2,0K 2016-10-25 16:56 test__pkce.pyc
-rw-rw-r-- 1 giampaolo giampaolo 7,4K 2016-10-25 16:56 test__pure_python_crypt.py
-rw-rw-r-- 1 giampaolo giampaolo 8,8K 2016-10-25 16:56 test__pure_python_crypt.pyc
-rw-rw-r-- 1 giampaolo giampaolo 2,7K 2016-10-25 16:56 test__pycrypto_crypt.py
-rw-rw-r-- 1 giampaolo giampaolo 3,3K 2016-10-25 16:56 test__pycrypto_crypt.pyc
-rw-rw-r-- 1 giampaolo giampaolo  28K 2016-10-25 16:56 test_service_account.py
-rw-rw-r-- 1 giampaolo giampaolo  22K 2016-10-25 16:56 test_service_account.pyc
-rw-rw-r-- 1 giampaolo giampaolo 7,4K 2016-10-25 16:56 test_tools.py
-rw-rw-r-- 1 giampaolo giampaolo 6,7K 2016-10-25 16:56 test_tools.pyc
-rw-rw-r-- 1 giampaolo giampaolo 6,1K 2016-10-25 16:56 test_transport.py
-rw-rw-r-- 1 giampaolo giampaolo 8,0K 2016-10-25 16:56 test_transport.pyc

[giampaolo]

I can fix it if I put this line before import tests:

sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))

Invoking tests as python -m tests instead of python tests\__main__.py also works. I don't particularly like neither of those though.

[giampaolo]

The problem here is that before tests were installed as an actual sub-package. The advantage of having tests inside psutil namespace was that the import was not ambiguous ("from psutil.tests import something"). Now we have import tests instead. Maybe we should revert this change and do something like this instead:

1. set PSUTIL_TESTING=1 env variable in the Makefile for all make test* targets.
2. from setup.py install psutil.tests package only if that variable is set

Not sure if there are better solutions. What do you think?

[jdufresne]

Another option is to do away with tests/__init__.py and tests/__main__.py entirely. It seems to mostly be duplicating unittest's default discovery logic. Tests already work by running:

$ python -m unittest discover tests

If preferred, this simple command could be wrapped in a script to make it shorter. I think this simplifies psutil's testing behavior and brings it more in line with default Python testing. I'm happy to make a PR for it if you agree. What do you think?

[giampaolo]

tests/__init__.py are test utilites (lots of them) shared amongst all test_*.py modules. We can't get rid of it.

[wiggin15]

We can maybe move the utilities to another file - say, psutil_tests.py or tests_common.py. This will be a big change, though.

[giampaolo]

I think keeping tests in psutil.tests namespace and installing them on make install via an env var is a good compromise.

[giampaolo]

The main point here should be avoiding to install tests in site-packages dir.

[giampaolo]

I reverted this PR and I stand by my original suggestion of keeping tests in psutil\tests namespace and install them from make.

[jdufresne]

If you give me some time, I can try to resolve this.

[giampaolo]

Sure. I suggest something along these lines:

• Makefile: set PSUTIL_INSTALL_TESTS=1 for make install target
• get it from os.environ in setup.py and install tests as a conditional

[giampaolo]

• update doc/READMEs where they mention tests can be run as python -m psutil.tests