Skip to content

Convert test decorators to pytest fixtures #7973

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

Merged
merged 3 commits into from
Feb 1, 2017
Merged
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
1 change: 0 additions & 1 deletion lib/matplotlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,6 @@ def _jupyter_nbextension_paths():
default_test_modules = [
'matplotlib.tests.test_png',
'matplotlib.tests.test_units',
'matplotlib.tests.test_widgets',
]
Copy link
Contributor

Choose a reason for hiding this comment

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

what's happening there?

Copy link
Member Author

Choose a reason for hiding this comment

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

When switching to the automatic fixture and dropping any @cleanups, the tests fail with nose because they don't get that setup done. However, see next comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for being dense, but why does this variable even exist? Why do we have a default whitelist of tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used for matplotlib.test(); in the next PR I just set it to the top-level modules.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to keep matplotlib.test()? Even if we do, can we just make matplotlib.test() fully rely on pytest's test discovery?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's why I pass only the top-level modules; but we still need to pass something because this works on the installed version and we need to hit both matplotlib and mpl_toolkits. I wouldn't be against deprecating matplotlib.test() and the top-level tests.py.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get it.

After the general conversion, do we expect nosetests to correctly run the tests?

  • If the answer is yes, I give up (I appreciate your efforts in making this work but conversion to a test suite that supports two runners is way too hard for me to follow properly).
  • If the answer is no, we already broke one way of running the tests. I honestly cannot imagine why we would need to keep backcompat for another one, and would thus just kill matplotlib.test() (... and tests.py at the same time).

Copy link
Member Author

Choose a reason for hiding this comment

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

The answer to the first question is no as far as our own tests are concerned. But matplotlib.test() and tests.py will work with pytest in #7974. I have no idea how many people may or may not be using that method, so I don't know whether we should remove it or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

So we already broke backcompat on our tests by having a requirement on pytest, and by making it not possible to run the test suite by running nosetests. I simply do not understand why we can't just break it a bit more to get rid of old warts... that we're planning to get rid of anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

Again, I'm not against it, but I think this discussion really belongs on #7974 where that code is actually changed.



Expand Down
4 changes: 4 additions & 0 deletions lib/matplotlib/sphinxext/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from __future__ import (absolute_import, division, print_function,
unicode_literals)

from matplotlib.tests.conftest import mpl_test_settings
45 changes: 45 additions & 0 deletions lib/matplotlib/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
from __future__ import (absolute_import, division, print_function,
unicode_literals)

import pytest

import matplotlib


@pytest.fixture(autouse=True)
def mpl_test_settings(request):
from matplotlib.testing.decorators import _do_cleanup

original_units_registry = matplotlib.units.registry.copy()
original_settings = matplotlib.rcParams.copy()

backend = None
backend_marker = request.keywords.get('backend')
if backend_marker is not None:
assert len(backend_marker.args) == 1, \
"Marker 'backend' must specify 1 backend."
backend = backend_marker.args[0]
prev_backend = matplotlib.get_backend()

style = 'classic'
style_marker = request.keywords.get('style')
if style_marker is not None:
assert len(style_marker.args) == 1, \
"Marker 'style' must specify 1 style."
style = style_marker.args[0]

matplotlib.testing.setup()
if backend is not None:
# This import must come after setup() so it doesn't load the default
# backend prematurely.
import matplotlib.pyplot as plt
plt.switch_backend(backend)
matplotlib.style.use(style)
try:
yield
finally:
if backend is not None:
import matplotlib.pyplot as plt
plt.switch_backend(prev_backend)
_do_cleanup(original_units_registry,
original_settings)
7 changes: 1 addition & 6 deletions lib/matplotlib/tests/test_agg.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@
from matplotlib.image import imread
from matplotlib.backends.backend_agg import FigureCanvasAgg as FigureCanvas
from matplotlib.figure import Figure
from matplotlib.testing.decorators import cleanup, image_comparison
from matplotlib.testing.decorators import image_comparison
from matplotlib import pyplot as plt
from matplotlib import collections
from matplotlib import path
from matplotlib import transforms as mtransforms


@cleanup
def test_repeated_save_with_alpha():
# We want an image which has a background color of bluish green, with an
# alpha of 0.25.
Expand Down Expand Up @@ -51,7 +50,6 @@ def test_repeated_save_with_alpha():
decimal=3)


@cleanup
def test_large_single_path_collection():
buff = io.BytesIO()

Expand All @@ -66,7 +64,6 @@ def test_large_single_path_collection():
plt.savefig(buff)


@cleanup
def test_marker_with_nan():
# This creates a marker with nans in it, which was segfaulting the
# Agg backend (see #3722)
Expand All @@ -79,7 +76,6 @@ def test_marker_with_nan():
fig.savefig(buf, format='png')


@cleanup
def test_long_path():
buff = io.BytesIO()

Expand Down Expand Up @@ -218,7 +214,6 @@ def process_image(self, padded_src, dpi):
ax.yaxis.set_visible(False)


@cleanup
def test_too_large_image():
fig = plt.figure(figsize=(300, 1000))
buff = io.BytesIO()
Expand Down
3 changes: 0 additions & 3 deletions lib/matplotlib/tests/test_animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import matplotlib as mpl
from matplotlib import pyplot as plt
from matplotlib import animation
from matplotlib.testing.decorators import cleanup


class NullMovieWriter(animation.AbstractMovieWriter):
Expand Down Expand Up @@ -109,7 +108,6 @@ def isAvailable(self):
# Smoke test for saving animations. In the future, we should probably
# design more sophisticated tests which compare resulting frames a-la
# matplotlib.testing.image_comparison
@cleanup
@pytest.mark.parametrize('writer, extension', WRITER_OUTPUT)
def test_save_animation_smoketest(tmpdir, writer, extension):
try:
Expand Down Expand Up @@ -148,7 +146,6 @@ def animate(i):
"see issues #1891 and #2679")


@cleanup
def test_no_length_frames():
fig, ax = plt.subplots()
line, = ax.plot([], [])
Expand Down
8 changes: 1 addition & 7 deletions lib/matplotlib/tests/test_artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@
import matplotlib.transforms as mtrans
import matplotlib.collections as mcollections
import matplotlib.artist as martist
from matplotlib.testing.decorators import image_comparison, cleanup
from matplotlib.testing.decorators import image_comparison


@cleanup
def test_patch_transform_of_none():
# tests the behaviour of patches added to an Axes with various transform
# specifications
Expand Down Expand Up @@ -60,7 +59,6 @@ def test_patch_transform_of_none():
assert e._transform == ax.transData


@cleanup
def test_collection_transform_of_none():
# tests the behaviour of collections added to an Axes with various
# transform specifications
Expand Down Expand Up @@ -127,7 +125,6 @@ def test_clipping():
ax1.set_ylim([-3, 3])


@cleanup
def test_cull_markers():
x = np.random.random(20000)
y = np.random.random(20000)
Expand Down Expand Up @@ -175,7 +172,6 @@ def test_hatching():
ax.set_ylim(0, 9)


@cleanup
def test_remove():
fig, ax = plt.subplots()
im = ax.imshow(np.arange(36).reshape(6, 6))
Expand Down Expand Up @@ -224,7 +220,6 @@ def test_default_edges():
ax4.add_patch(pp1)


@cleanup
def test_properties():
ln = mlines.Line2D([], [])
with warnings.catch_warnings(record=True) as w:
Expand All @@ -234,7 +229,6 @@ def test_properties():
assert len(w) == 0


@cleanup
def test_setp():
# Check empty list
plt.setp([])
Expand Down
Loading