Skip to content

V1.4.x - Remove test dependencies from install_requires #4188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ env:
- secure: E7OCdqhZ+PlwJcn+Hd6ns9TDJgEUXiUNEI0wu7xjxB2vBRRIKtZMbuaZjd+iKDqCKuVOJKu0ClBUYxmgmpLicTwi34CfTUYt6D4uhrU+8hBBOn1iiK51cl/aBvlUUrqaRLVhukNEBGZcyqAjXSA/Qsnp2iELEmAfOUa92ZYo1sk=
- secure: "dfjNqGKzQG5bu3FnDNwLG8H/C4QoieFo4PfFmZPdM2RY7WIzukwKFNT6kiDfOrpwt+2bR7FhzjOGlDECGtlGOtYPN8XuXGjhcP4a4IfakdbDfF+D3NPIpf5VlE6776k0VpvcZBTMYJKNFIMc7QPkOwjvNJ2aXyfe3hBuGlKJzQU="
- BUILD_DOCS=false
- TEST_ARGS=--no-pep8
- NUMPY=numpy
- NPROC=2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need this for the build matrix to work right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, is line 26 working because something else in there depends on numpy?

- TEST_ARGS=--omit-pep8

language: python

Expand All @@ -39,7 +39,7 @@ matrix:
- python: 3.3
- python: 3.4
- python: 2.7
env: TEST_ARGS=--pep8
env: TEST_ARGS=--pep8-only
- python: 2.7
env: BUILD_DOCS=true
- python: "nightly"
Expand Down Expand Up @@ -72,7 +72,7 @@ install:
# Neihter is installed as a dependency of IPython since they are not used by the IPython console.
- |
if [[ $BUILD_DOCS == true ]]; then
pip install $PRE numpydoc ipython jsonschema mistune
pip install $PRE numpydoc ipython jsonschema mistune mock
pip install -q $PRE linkchecker
wget https://github.com/google/fonts/blob/master/ofl/felipa/Felipa-Regular.ttf?raw=true -O Felipa-Regular.ttf
wget http://mirrors.kernel.org/ubuntu/pool/universe/f/fonts-humor-sans/fonts-humor-sans_1.0-1_all.deb
Expand All @@ -96,9 +96,10 @@ script:
- |
if [[ $BUILD_DOCS == false ]]; then
export MPL_REPO_DIR=$PWD # needed for pep8-conformance test of the examples
mkdir ../tmp_test_dir
cd ../tmp_test_dir
gdb -return-child-result -batch -ex r -ex bt --args python ../matplotlib/tests.py -s --processes=$NPROC --process-timeout=300 $TEST_ARGS
# mkdir ../tmp_test_dir
# cd ../tmp_test_dir
# gdb -return-child-result -batch -ex r -ex bt --args python ../matplotlib/tests.py -s --processes=$NPROC --process-timeout=300 $TEST_ARGS
gdb -return-child-result -batch -ex r -ex bt --args python setup.py test --nocapture --nose-verbose --processes=8 --process-timeout=300 $TEST_ARGS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does '--nocapture' do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nm, read the rest of the PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please change this so that it does not generate log files larger than travis will display again?

else
cd doc
python make.py html --small --warningsaserrors
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ jdh_doc_snapshot:


test:
${PYTHON} tests.py
${PYTHON} setup.py test


test-coverage:
${PYTHON} tests.py --with-coverage --cover-package=matplotlib
${PYTHON} setup.py test --with-coverage --cover-package=matplotlib


2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Testing

After installation, you can launch the test suite::

python tests.py
python setup.py tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please document both methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just saw that this should be python setup.py test, so there is a mis-spelling here as well


Consider reading http://matplotlib.org/devel/coding_guide.html#testing for
more information.
4 changes: 2 additions & 2 deletions doc/devel/release_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ A guide for developers who are doing a matplotlib release.
Testing
=======

* Run all of the regression tests by running the `tests.py` script at
the root of the source tree.
* Run all of the regression tests by running ``python setup.py tests`` script
at the root of the source tree.

* Run :file:`unit/memleak_hawaii3.py` and make sure there are no
memory leaks
Expand Down
50 changes: 33 additions & 17 deletions doc/devel/testing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,24 @@ Running the tests
-----------------

Running the tests is simple. Make sure you have nose installed and run
the script :file:`tests.py` in the root directory of the distribution.
The script can take any of the usual `nosetest arguments`_, such as
the setup script's ``test`` command::

=================== ===========
``-v`` increase verbosity
``-d`` detailed error messages
``--with-coverage`` enable collecting coverage information
=================== ===========
python setup.py test

in the root directory of the distribution. The script takes a set of
commands, such as:

======================== ===========
``--pep8-only`` pep8 checks
``--omit-pep8`` Do not perform pep8 checks
``--nocapture`` do not capture stdout (nosetests)
``--nose-verbose`` be verbose (nosetests)
``--processes`` number of processes (nosetests)
``--process-timeout`` process timeout (nosetests)
``--with-coverage`` with coverage
``--detailed-error-msg`` detailed error message (nosetest)
``--tests`` comma separated selection of tests (nosetest)
======================== ===========

Additionally it is possible to run only coding standard test or disable them:

Expand All @@ -57,28 +67,34 @@ To run a single test from the command line, you can provide a
dot-separated path to the module followed by the function separated by
a colon, e.g., (this is assuming the test is installed)::

python tests.py matplotlib.tests.test_simplification:test_clipping

If you want to run the full test suite, but want to save wall time try running the
tests in parallel::
python setup.py test --tests=matplotlib.tests.test_simplification:test_clipping

python ../matplotlib/tests.py -sv --processes=5 --process-timeout=300
If you want to run the full test suite, but want to save wall time try
running the tests in parallel::

as we do on Travis.ci.
python setup.py test --nocapture --nose-verbose --processes=5 --process-timeout=300


An alternative implementation that does not look at command line
arguments works from within Python::
arguments works from within Python is to run the tests from the
matplotlib library function :func:`matplotlib.test`::

import matplotlib
matplotlib.test()

.. hint::

You might need to install nose for this::

pip install nose


.. _`nosetest arguments`: http://nose.readthedocs.org/en/latest/usage.html


Running tests by any means other than `matplotlib.test()`
does not load the nose "knownfailureif" (Known failing tests) plugin,
causing known-failing tests to fail for real.
Running tests by any means other than `matplotlib.test()` does not
load the nose "knownfailureif" (Known failing tests) plugin, causing
known-failing tests to fail for real.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this statement still true?


Writing a simple test
---------------------
Expand Down
120 changes: 120 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# This needs to be the very first thing to use distribute
from distribute_setup import use_setuptools
use_setuptools()
from setuptools.command.test import test as TestCommand

import sys

Expand Down Expand Up @@ -121,6 +122,121 @@
'Topic :: Scientific/Engineering :: Visualization',
]


class NoseTestCommand(TestCommand):
"""Invoke unit tests using nose after an in-place build."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "an in-place build" here? Does this mean one can't run the tests after a normal python setup.py install?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure what I mean with "an in-place build" here.

You should be able to run the tests after a python setup.py install.

I guess the original line of thought for this pull request has been lost upon rebasing. This will make things more clear. I started this pull request, because maptlotlib introduced a dependency on nose somewhat after version matplotlib 1.2. Nose is however not a normal runtime dependency, in fact it is only needed for executing and running the tests. So I would like matplotlib to follow the standard practice here of not specifying this as a runtime dependency.

Distutils / setuptools has a specific parameter for test dependencies, this pull request moves the test dependencies there. Apart from removing nose from the runtime dependencies (i.e. that are installed when you pip install matplotlib) this has 2 consequences:

  1. When running python setup.py test the test dependencies are installed as eggs into the project folder (not into the virtual env) and
  2. a test command must be hooked in setup.py to execute the tests to execute the tests this way

You can still run tests with nose installed into the virtualenv as before (then no egg is put into the project directory, you just have to specifically pip install nose mock). And you can also execute the tests with the library function (i.e. from the python prompt).


description = "Invoke unit tests using nose after an in-place build."
user_options = [
("pep8-only", None, "pep8 checks"),
("omit-pep8", None, "Do not perform pep8 checks"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these standard names? If not can you revert these to the flags we were using before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this so long ago I can hardly remember if I changed those names (I presume I did) . Probably I had the impression that the flags from the script runner were ambiguous.

("nocapture", None, "do not capture stdout (nosetests)"),
("nose-verbose", None, "be verbose (nosetests)"),
("processes=", None, "number of processes (nosetests)"),
("process-timeout=", None, "process timeout (nosetests)"),
("with-coverage", None, "with coverage"),
("detailed-error-msg", None, "detailed error message (nosetest)"),
("tests=", None, "comma separated selection of tests (nosetest)"),
]

def initialize_options(self):
self.pep8_only = None
self.omit_pep8 = None

# parameters passed to nose tests
self.processes = None
self.process_timeout = None
self.nose_verbose = None
self.nocapture = None
self.with_coverage = None
self.detailed_error_msg = None
self.tests = None

def finalize_options(self):
self.test_args = []
if self.pep8_only:
self.pep8_only = True
if self.omit_pep8:
self.omit_pep8 = True

if self.pep8_only and self.omit_pep8:
from distutils.errors import DistutilsOptionError
raise DistutilsOptionError(
"You are using several options for the test command in an "
"incompatible manner. Please use either one of --pep8-only,"
"--omit-pep8"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably have a space here. It would render as "one of --pep8-only,--omit-pep8"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammatically, it should be "only one of X, Y" or "either X or Y".

)

if self.processes:
self.test_args.append("--processes={prc}".format(
prc=self.processes))

if self.process_timeout:
self.test_args.append("--process-timeout={tout}".format(
tout=self.process_timeout))

if self.nose_verbose:
self.test_args.append("--verbose")

if self.nocapture:
self.test_args.append("--nocapture")

if self.with_coverage:
self.test_args.append("--with-coverage")

if self.detailed_error_msg:
self.test_args.append("-d")

if self.tests:
self.test_args.append("--tests={names}".format(names=self.tests))


def run(self):
if self.distribution.install_requires:
self.distribution.fetch_build_eggs(
self.distribution.install_requires)
if self.distribution.tests_require:
self.distribution.fetch_build_eggs(self.distribution.tests_require)

self.announce('running unittests with nose')
self.with_project_on_sys_path(self.run_tests)


def run_tests(self):
try:
import matplotlib
matplotlib.use('agg')
import nose
from matplotlib.testing.noseclasses import KnownFailure
from matplotlib import default_test_modules as testmodules
from matplotlib import font_manager
import time
# Make sure the font caches are created before starting any possibly
# parallel tests
if font_manager._fmcache is not None:
while not os.path.exists(font_manager._fmcache):
time.sleep(0.5)
plugins = [KnownFailure]

# Nose doesn't automatically instantiate all of the plugins in the
# child processes, so we have to provide the multiprocess plugin
# with a list.
from nose.plugins import multiprocess
multiprocess._instantiate_plugins = plugins

if self.omit_pep8:
testmodules.remove('matplotlib.tests.test_coding_standards')
elif self.pep8_only:
testmodules = ['matplotlib.tests.test_coding_standards']

nose.main(addplugins=[x() for x in plugins],
defaultTest=testmodules,
argv=['nosetests'] + self.test_args,
exit=False)
except ImportError:
sys.exit(-1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have an error message of some sort?



# One doesn't normally see `if __name__ == '__main__'` blocks in a setup.py,
# however, this is needed on Windows to avoid creating infinite subprocesses
# when using multiprocessing.
Expand All @@ -135,6 +251,7 @@
package_dir = {'': 'lib'}
install_requires = []
setup_requires = []
tests_require = []
default_backend = None

# Go through all of the packages and figure out which ones we are
Expand Down Expand Up @@ -195,6 +312,7 @@
package_data[key] = list(set(val + package_data[key]))
install_requires.extend(package.get_install_requires())
setup_requires.extend(package.get_setup_requires())
tests_require.extend(package.get_tests_require())

# Write the default matplotlibrc file
if default_backend is None:
Expand Down Expand Up @@ -254,11 +372,13 @@
# List third-party Python packages that we require
install_requires=install_requires,
setup_requires=setup_requires,
tests_require=tests_require,

# matplotlib has C/C++ extensions, so it's not zip safe.
# Telling setuptools this prevents it from doing an automatic
# check for zip safety.
zip_safe=False,
cmdclass={'test': NoseTestCommand},

**extra_args
)
8 changes: 7 additions & 1 deletion setupext.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,12 @@ def get_setup_requires(self):
"""
return []

def get_tests_require(self):
"""
Get a list of Python packages that we require for executing tests.
"""
return []

def _check_for_pkg_config(self, package, include_file, min_version=None,
version=None):
"""
Expand Down Expand Up @@ -694,7 +700,7 @@ def get_package_data(self):
'sphinxext/tests/tinypages/_static/*',
]}

def get_install_requires(self):
def get_tests_require(self):
requires = ['nose>=%s' % self.nose_min_version]
if not sys.version_info >= (3, 3):
requires += ['mock']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this also have a sphinx dep?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are matplotlib tests depending on sphinx? If yes, Sphinx should go here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we now have tests for the sphinx extensions.

Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ commands =
{envpython} {toxinidir}/tests.py --processes=-1 --process-timeout=300
deps =
nose
mock
numpy