Prepare for cross-framework test suite #6920
Conversation
|
👍 from my first quick read of the changes |
6443d98 to
634cdf3
Compare
|
Small fix: |
| try: | ||
| import nose | ||
| try: | ||
| from unittest import mock |
There was a problem hiding this comment.
mock is used in other tests; not sure this test should be removed.
There was a problem hiding this comment.
Check is not removed, it is just moved to check_deps function
7dc73af to
df0d119
Compare
9d7ef8d to
362c271
Compare
|
This needs a rebase |
- introduced `xfail(reason)` function
- uses: `raise KnownFailureTest(msg)` -> `xfail(reason)`
- same name and signature as in pytest
- introduced `skip(reason)` function
- uses: `raise SkipTest(msg)` -> `skip(reason)`
- same name and signature as in pytest
- introduced `skipif(condition, reason=None)` decorator
- uses: replaces `def func(): if condition: skip()`
- same name and signature as in pytest
- can be used with functions, classes, and methods
- supports string condition (evaluated at runtime)
- moved nose related code to `testing.nose` submodule
- plugins in `testing.nose.plugins` submodule
- decorators implementation in `testing.nose.decorators`
(interface is still in `testing.decorators`, implementation will
have been chosen at runtime according to used test framework)
- `matplotlib.test` function unifications
- `tests.py` now uses `matplotlib.test()`
362c271 to
f20efb9
Compare
|
Ping |
| @@ -1,15 +1,3 @@ | |||
| class KnownFailureTest(Exception): | |||
There was a problem hiding this comment.
I would rather not move this for compatibility reasons.
There was a problem hiding this comment.
Yeah, I am pretty sure other projects actually uses our KnownFailure stuff.
On Wed, Aug 17, 2016 at 1:15 PM, Thomas A Caswell notifications@github.com
wrote:
In lib/matplotlib/testing/exceptions.py
#6920 (comment):@@ -1,15 +1,3 @@
-class KnownFailureTest(Exception):I would rather not move this for compatibility reasons.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/matplotlib/matplotlib/pull/6920/files/f20efb99d67e581f18f8fd58db07d6279528fcbd#r75165500,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARy-A6W8iBehFuv_ltVBw91aSSmfrI6ks5qg0GmgaJpZM4Jemgm
.
There was a problem hiding this comment.
I cannot agree with this for two reasons:
- PRs that relay on
raise KnownFailureTestshould not pass after this PR ever, and this change will force them to switch onxfail - Other projects must not relay on matplotlib testing features, it is their problem if they are
I can understand backward compatibility on matplotlib core features, but with removing nose support all this stuff will gone. I have already tried my best to make dual nose-pytest support, but leaving nose-specific stuff on the current places will blow something in any time in the future.
There was a problem hiding this comment.
Just to note, matplotlib and many other projects depend on some of numpy's
testing features. By your logic, that shouldn't be done and every project
should implement their own numerical comparison tools?
I am not exactly sure how to proceed here.
On Wed, Aug 17, 2016 at 1:31 PM, Nikita Kniazev notifications@github.com
wrote:
In lib/matplotlib/testing/exceptions.py
#6920 (comment):@@ -1,15 +1,3 @@
-class KnownFailureTest(Exception):I cannot agree with this for two reasons:
- PRs that relay on raise KnownFailureTest should not pass after this
PR ever, and this change will force them to switch on xfail- Other projects must not relay on matplotlib testing features, it is
their problem if they areI can understand backward compatibility on matplotlib core features, but
with removing nose support all this stuff will gone. I have already tried
my best to make dual nose-pytest support, but leaving nose-specific stuff
on the current places will blow something in any time in the future.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/matplotlib/matplotlib/pull/6920/files/f20efb99d67e581f18f8fd58db07d6279528fcbd#r75168432,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARy-HYvL9iMog7FVqaAIeBGMR-v5ev-ks5qg0WIgaJpZM4Jemgm
.
There was a problem hiding this comment.
By my logic - anything that is not a part of the public API can be changed or even removed at any time, so everyone who did something like using KnownFailureTest makes it on its own risk.
There was a problem hiding this comment.
Numpy's tools are useful for testing using numpy arrays, so I don't find referencing them to be a compelling argument. The exceptions removed here are not unique to matplotlib functionality. It was a bad idea to for people to use them from us in the first place, and are trivially recreated for anyone down stream who happen to use them.
|
'powercycled to try and trigger coveralls |
|
Merging this as it is blocking progress for @story645 Independent of if the testing code should be part of the public API, people are using it that way (and to my knowledge) we have not been actively discouraging it. We are going to have to eventually shim these back or document the API changes. |
|
and the travis failure is an install failure on the basemap dependency. |
I have made a separated PR for this commit because I need a review for it (this is the base for other changes related to pytest framework support).
Changes:
xfail(reason)functionraise KnownFailureTest(msg)->xfail(reason)skip(reason)functionraise SkipTest(msg)->skip(reason)skipif(condition, reason=None)decoratordef func(): if condition: skip()testing.nosesubmoduletesting.nose.pluginssubmoduletesting.nose.decorators(interface is still in
testing.decorators, implementation willhave been chosen at runtime according to used test framework)
matplotlib.testfunction unificationstests.pynow usesmatplotlib.test()