Pytest documentation + build tweaks #8026

QuLogic · 2017-02-05T23:00:59Z

I've added the developer documentation for setting up pytest and removed a few more cases of nose in the setup, package metadata, etc.

For the tweaks:

Fix tests.py when run with a module path instead of a file path.
Close figures before running image_comparison because that decorator expects all figures when it's done to correspond to figures from the test, but if never checks for/ignores any already open figures.
Delete test_mouseclicks.py example because it's getting collected and run by pytest and since it's not really written as a test it causes an extra figure to be opened which breaks any image tests if run directly (modulo the previous fix.) Also copy the use of event.dblclick into the documentation so it's not gone completely.
Use importorskip everywhere so that the skip reasons are always the same, since it's used in some places and not others.

Fixes #8015.

QuLogic · 2017-02-05T23:21:32Z

Added one more tweak to use importorskip everywhere so that the skip reasons are always the same, since it's used in some places and not others.

QuLogic · 2017-02-06T01:20:21Z

I've added the developer documentation for setting up pytest and removed a few more cases of nose in the setup, package metadata, etc.

anntzer · 2017-02-06T07:30:37Z

Could we document the pytest interface rather than tests.py? I think it basically comes down to documenting -m "not network" instead of --no-network, the rest is the same.

After a quick look at tests.py, it also seems that there are some customizations (warnings filters, recursion limit) which should perhaps be moved to conftest.py?

dstansby · 2017-02-06T15:10:48Z

tox.ini

@@ -12,7 +12,6 @@ commands =
    sh -c 'rm -f $HOME/.matplotlib/fontList*'


Add python version 3.6 to envlist above

dstansby · 2017-02-06T15:13:26Z

lib/matplotlib/tests/test_axes.py

-        import pandas as pd
-    except ImportError:
-        pytest.skip("Pandas not installed")
+    pd = pytest.importorskip('pandas')


Is there a minimum required version of pandas needed for the pandas tests to work?

It doesn't appear that there is. Creating a Series from a list and DataFrame a from a dictionary of lists/arrays should work with versions of pandas several years old at this point.

dstansby · 2017-02-06T15:16:38Z

examples/event_handling/test_mouseclicks.py

@@ -1,32 +0,0 @@
-from __future__ import print_function


~~Also need to get rid of exammples/event_handling/test_mouseclicks.rst~~

Isn't that file autogenerated from the example? It's not in version control as far as I can see

🤦‍♂️ so it is

dstansby · 2017-02-06T15:20:54Z

doc/devel/testing.rst


  import matplotlib
  matplotlib.test()

 .. hint::

-   To run the tests you need to install nose and mock if using python 2.7::
+   To run the tests you need to install pytest and mock if using python 2.7::


Everything in this hint is covered above, so I think it can just be deleted

dstansby · 2017-02-06T15:22:27Z

doc/devel/testing.rst

-documentation for supported arguments. Some of the more important ones are given
-here:
+Additional arguments are passed on to pytest. See the pytest documentation for
+supported arguments. Some of the more important ones are given here:


Might be good to put a link to the pytest documents that list supported arguments

dstansby · 2017-02-06T15:24:09Z

doc/devel/testing.rst


-  python tests.py matplotlib.tests.test_simplification:test_clipping
+  python tests.py matplotlib.tests.test_simplification::test_clipping


Is there any point documenting the method where tests have to be installed, if the method below (which doesn't require installed tests) works?

I don't know; does anyone use this method?

On further though, using non-installed tests might be broken when trying mpl_toolkits tests because they import stuff from matplotlib.tests. I can move things around to get that working again.

Since this decorator assumes that the figures available after are the ones to check, it should close any existing figures or it will get the wrong count.

The name is getting collected as a test by pytest when it isn't one, and it isn't much of an example anyway.

It's used in some places, but not all.

This may or may not become a requirement in the future.

* Check for installed test data only if testing installed package. * Remove relative import out of tests. * Replace assert_str_equal with plain assert (pytest does diffing already).

QuLogic · 2017-02-16T09:10:11Z

I updated the documentation to follow the requests here and also moved some bits around so that tests work without being installed. Everything but the determinism test in the SVG backend works (because that one imports itself, which doesn't exist when not installed); not too sure how to fix that one, but it's not a high priority since not-installed tests never worked before.

NelleV

Here are general comments on the documentation part.
This are minor comments and should not prevent the pull requests from being merged. These are just overall suggestions of improvements.

I'll review the rest of the PR.

NelleV · 2017-02-16T18:56:02Z

INSTALL

@@ -106,12 +106,10 @@ or example code.
 If you want to try the many demos that come in the matplotlib source
 distribution, download the :file:`*.tar.gz` file and look in the
 :file:`examples` subdirectory.
-To run the test suite, copy the :file:`lib\\matplotlib\\tests` and
+To run the test suite, extract the :file:`lib\\matplotlib\\tests` or


I don't understand this sentence.

I split things into bullet points; hopefully that's clearer.

NelleV · 2017-02-16T18:59:43Z

README.rst

-more information. Note that the test suite requires nose and on Python 2.7 mock
-which are not installed by default. Please install with pip or your package
-manager of choice.
+more information. Note that the test suite requires pytest and on Python 2.7


I think this sentence would be better with comas (for the understanding):
Note that the test suite requires pytest and, on Python 2.7, mock.
The "which are not installed by default seems redundant.

I would be happy to push a fix to your branch. Let me know if you prefer this option.

NelleV · 2017-02-16T18:59:52Z

ci/conda_recipe/meta.yaml

@@ -30,7 +30,7 @@ requirements:
    - freetype 2.6*
    - msinttypes  # [win]
    - cycler >=0.10
-    - nose
+    - pytest >=3.0.0


NelleV · 2017-02-16T19:00:11Z

doc/devel/contributing.rst

-  <https://docs.python.org/dev/library/unittest.mock.html>`_ (if python < 3.3), `Ghostscript
-  <https://www.ghostscript.com/>`_, `Inkscape <https://inkscape.org>`_
+  **Additional dependencies for testing**: pytest_ (version 3.0 or later),
+  mock_ (if python < 3.3), Ghostscript_, Inkscape_


Python 3 < 3.3 isn't supported but matplotlib, so might make more sense to change this to "if python 2.7"

NelleV · 2017-02-16T19:05:17Z

doc/devel/testing.rst

-please ignore it while we consolidate our testing to these locations.)
-
-.. _nose: https://nose.readthedocs.io/en/latest/
+Matplotlib has a testing infrastructure based on pytest_, making it easy to


Here are suggestions:

Matplotlib's testing infrastucture relies on pytest_. Test files are place in [path to the folder], and testing utility functions are in :mod:`matplotlib.testing`.

I'd drop the "making it easy to write new tests", as this is a judgement call that I also totally disagree with.
I'd also remove the "There is other old testing cruft around…"

Changed; also switched to the file paths instead of modules since we're talking about developers who probably want to edit the existing files.

NelleV · 2017-02-16T19:07:30Z

doc/devel/testing.rst


-  python tests.py matplotlib.tests.test_simplification:test_clipping
+  py.test lib/matplotlib/tests/test_simplification.py::test_clipping


NelleV · 2017-02-16T19:07:54Z

doc/devel/testing.rst

-  python tests.py matplotlib.tests.test_simplification:test_clipping
+  py.test lib/matplotlib/tests/test_simplification.py::test_clipping
+
+or, if tests are installed, a dot-separated path to the module, optionally


I would remove this section. We only need to document one way to do this.

NelleV · 2017-02-16T19:11:14Z

doc/devel/testing.rst

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

  import matplotlib
  matplotlib.test()


I'd remove this section from our documentation. I think we should document (and have) only one way to run the tests.

attn @tacaswell (as I believe Thomas will disagree with me on that one).

NelleV · 2017-02-16T19:14:28Z

doc/devel/testing.rst

-      """
-      fig = figure()
-      ...
+Tests that have side effects that need to be cleaned up, such as created


That paragraph is very unclear to me. Here is a suggestions.

"Some tests have side effects that need to be cleaned up after their execution (such as created figures or modifiec rc params). The pytest fixture XXX will automatically clean these up:

add short example showing how to use the fixture"

There's no need for an example because the fixture gets used automatically; I've swapped the text around as suggested.

NelleV · 2017-02-16T19:15:17Z

doc/users/event_handling.rst

@@ -29,13 +29,13 @@ connect your function to the event manager, which is part of the
 example that prints the location of the mouse click and which button
 was pressed::

-    fig = plt.figure()
-    ax = fig.add_subplot(111)
+    fig, ax = plt.subplots()


madphysicist · 2017-02-16T20:41:13Z

Thanks for doing this. This is very helpful. Both the docs and switching to pytest. There is a bit of a learning curve with it, but it seems to be much more flexible than nose.

* Prefer py.test over tests.py * Remove redundant hint * Add link to further pytest documentation * Tweak some sentence structure

The only difference between the two 3.5 builds is one uses tests.py and the other uses py.test. But the 2.7, 3.4, and macOS builds all use tests.py, to this is a waste of resources.

QuLogic · 2017-02-18T00:32:40Z

I've removed an extra 3.5 build on Travis because it's pretty redundant; hopefully that will get us to finished CI just a little bit faster.

NelleV · 2017-02-18T20:21:38Z

LGTM 👍

anntzer · 2017-02-19T00:55:03Z

doc/devel/testing.rst

-An alternative implementation that does not look at command line
-arguments works from within Python is to run the tests from the
-matplotlib library function :func:`matplotlib.test`::
+  PYTHONHASHSEED=0 py.test --verbose -n 5


Can we just specify a minimum version of pytest-xdist where this is no longer an issue? It looks like it is related to https://bitbucket.org/pytest-dev/pytest/issues/346/pytest-xdist-and-python-33-is-sort-of, which has been resolved in 2013.

Unfortunately, it's not fixed yet: pytest-dev/pytest#920

I just PR'd the same fix to the pandas build, so I know that recent versions still don't work.

dstansby

I think this looks ready to go! I've just left a couple of comments for things that might need tweaking in the docs; let me know once you've read and/or changed them, and I will merge.

dstansby · 2017-02-19T17:01:37Z

INSTALL

+   `mock <https://pypi.python.org/pypi/mock>`_, Pillow, MiKTeX, GhostScript,
+   ffmpeg, avconv, mencoder, ImageMagick, and `Inkscape
+   <https://inkscape.org/>`_;
+ * run ``py.test path\\to\\tests\\directory``.


According to Introduce pytest command as recommended entry point pytest-dev/pytest#1629 (comment) , pytest is preferred over py.test as of 3.0

I don't think you need the extra path\\to\\tests\\directory bit; if you do then line 23 of README.rst should be made consistent

This is about separate uninstalled tests; line 23 in README.rst is about running from the full source directory.

dstansby · 2017-02-19T17:03:12Z

README.rst

@@ -20,17 +20,16 @@ Testing

 After installation, you can launch the test suite::

-  python tests.py
+  py.test


See previous comment about py.test vs. pytest

dstansby · 2017-02-19T17:04:57Z

doc/devel/contributing.rst

-  <https://docs.python.org/dev/library/unittest.mock.html>`_ (if python < 3.3), `Ghostscript
-  <https://www.ghostscript.com/>`_, `Inkscape <https://inkscape.org>`_
+  **Additional dependencies for testing**: pytest_ (version 3.0 or later),
+  mock_ (if python < 3.3), Ghostscript_, Inkscape_


Python 3 < 3.3 isn't supported but matplotlib, so might make more sense to change this to "if python 2.7"

dstansby · 2017-02-19T17:06:01Z

doc/devel/testing.rst

-Running the tests is simple. Make sure you have nose installed and run::
+Running the tests is simple. Make sure you have pytest installed and run::
+
+   py.test


See above comment about py.test vs. pytest

dstansby · 2017-02-19T17:07:02Z

doc/devel/testing.rst

+or, if tests are installed, a dot-separated path to the module, optionally
+followed by the function separated by two colons, such as::
+
+  py.test --pyargs matplotlib.tests.test_simplification::test_clipping

 If you want to run the full test suite, but want to save wall time try


What is "wall time"?

Time on a clock on the wall, as opposed to cpu time.

dstansby · 2017-02-20T09:40:18Z

Since all the lines I commented on are technically true and easily changed later, I'm going to merge (finally!), thanks for all the work on this @QuLogic 👍

QuLogic added Documentation topic: testing labels Feb 5, 2017

QuLogic added this to the 2.1 (next point release) milestone Feb 5, 2017

QuLogic changed the title ~~Pytest tweaks~~ Pytest documentation + build tweaks Feb 6, 2017

QuLogic force-pushed the pytest-tweaks branch from 14598ad to 61bb7bf Compare February 6, 2017 01:24

dstansby requested changes Feb 6, 2017

View reviewed changes

QuLogic added 12 commits February 16, 2017 01:33

TST: Close figures before image comparison.

bdb8682

Since this decorator assumes that the figures available after are the ones to check, it should close any existing figures or it will get the wrong count.

Remove test_mouseclicks and copy some bits to docs.

b71447f

The name is getting collected as a test by pytest when it isn't one, and it isn't much of an example anyway.

Use pytest.importorskip for consistent skip message.

c632e41

It's used in some places, but not all.

Remove nose from tox environment.

113d079

Document markers used by pytest.

9d36e98

This may or may not become a requirement in the future.

Fix tests.py when passing modules instead of paths.

d9fbe8d

Update documentation to use pytest for testing.

244806d

Change setup to require pytest instead of nose.

25192ea

Remove multiprocess nose stuff.

50eaad6

TST: Add 3.6 to tox.ini.

4529002

Move conftest.py so it works with uninstalled tests.

5bc91c7

Fix running of tests without installation.

a4427a1

* Check for installed test data only if testing installed package. * Remove relative import out of tests. * Replace assert_str_equal with plain assert (pytest does diffing already).

QuLogic force-pushed the pytest-tweaks branch from 61bb7bf to 4fd85f1 Compare February 16, 2017 08:15

NelleV reviewed Feb 16, 2017

View reviewed changes

QuLogic added 2 commits February 17, 2017 19:00

Update pytest documentation based on review.

6a0c3ee

* Prefer py.test over tests.py * Remove redundant hint * Add link to further pytest documentation * Tweak some sentence structure

TST: Remove extra Travis build.

29c1308

The only difference between the two 3.5 builds is one uses tests.py and the other uses py.test. But the 2.7, 3.4, and macOS builds all use tests.py, to this is a waste of resources.

QuLogic force-pushed the pytest-tweaks branch from 4fd85f1 to 29c1308 Compare February 18, 2017 00:27

NelleV changed the title ~~Pytest documentation + build tweaks~~ [MRG+1] Pytest documentation + build tweaks Feb 18, 2017

NelleV approved these changes Feb 18, 2017

View reviewed changes

anntzer reviewed Feb 19, 2017

View reviewed changes

QuLogic requested a review from tacaswell February 19, 2017 02:00

dstansby approved these changes Feb 19, 2017

View reviewed changes

dstansby changed the title ~~[MRG+1] Pytest documentation + build tweaks~~ Pytest documentation + build tweaks Feb 20, 2017

dstansby merged commit 0d5a02a into matplotlib:master Feb 20, 2017

QuLogic deleted the pytest-tweaks branch February 20, 2017 21:53

anntzer mentioned this pull request Feb 21, 2017

Deprecate assert_str_equal #8112

Closed

		@@ -12,7 +12,6 @@ commands =
		sh -c 'rm -f $HOME/.matplotlib/fontList*'


		python tests.py matplotlib.tests.test_simplification:test_clipping
		python tests.py matplotlib.tests.test_simplification::test_clipping


		python tests.py matplotlib.tests.test_simplification:test_clipping
		py.test lib/matplotlib/tests/test_simplification.py::test_clipping

Pytest documentation + build tweaks #8026

Pytest documentation + build tweaks #8026

Conversation

QuLogic commented Feb 5, 2017 • edited Loading

QuLogic commented Feb 5, 2017

QuLogic commented Feb 6, 2017

anntzer commented Feb 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstansby Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuLogic commented Feb 16, 2017

NelleV left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madphysicist commented Feb 16, 2017

QuLogic commented Feb 18, 2017

NelleV commented Feb 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstansby left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstansby commented Feb 20, 2017

QuLogic commented Feb 5, 2017 •

edited

Loading

dstansby Feb 6, 2017 •

edited

Loading