Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Prepare for cross-framework test suite#6920
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Prepare for cross-framework test suite#6920
Uh oh!
There was an error while loading.Please reload this page.
Conversation
👍 from my first quick read of the changes |
6443d98 to634cdf3CompareSmall fix: |
| try: | ||
| import nose | ||
| try: | ||
| from unittest import mock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
mock is used in other tests; not sure this test should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Check is not removed, it is just moved tocheck_deps function
7dc73af todf0d119Compare9d7ef8d to362c271CompareThis 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 tof20efb9ComparePing |
| @@ -1,15 +1,3 @@ | |||
| class KnownFailureTest(Exception): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would rather not move this for compatibility reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, I am pretty sure other projects actually uses our KnownFailure stuff.
On Wed, Aug 17, 2016 at 1:15 PM, Thomas A Caswellnotifications@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 Kniazevnotifications@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 usingKnownFailureTest makes it on its own risk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 ofif 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()