Skip to content

Migration to Py.test testing framework #6731

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
34 of 39 tasks
Kojoley opened this issue Jul 12, 2016 · 23 comments
Closed
34 of 39 tasks

Migration to Py.test testing framework #6731

Kojoley opened this issue Jul 12, 2016 · 23 comments

Comments

@Kojoley
Copy link
Member

Kojoley commented Jul 12, 2016

This task has been started in PR #5405, but ended without success, so I have decided to split it to multiple stages:

  1. Make pytest compatible with the current test suite
  2. Migration to pytest
    • Add new build under allow_failures with pytest
      • Appveyor
      • Travis
      • Use pytest-cov for coverage reports
    • Replace nose in all builds with pytest
  3. Post-migration
    • Rename setUp -> setup_method and other methods to pytest analogs
      • Do not inherit tests from unitest.UnitTest class
      • Remove setUp/tearDown workaround
    • Rewrite image_comparison decorator (currently it slowdowns the collection phase due to baseline images copying) or use pytest-mpl plugin (needs some improvements, see paragraph below)
    • Rewrite all assertations to pure assert
    • Get away from cleanup decorator
    • Replace knownfailif with mark.xfail
    • Replace raise nose.SkipTest(...) with xfail(...) call or mark.skipif decorator
  4. Enhancements
    • Replace setup_method and etc methods with fixtures
    • Rewrite test_delaunay.make_all_2d_testfuncs
    • Use pytest-pep8 plugin instead of test_coding_standards
    • Use pytest-mock plugin instead of mock and unittest.mock

Tests entry point (raised in #5405 (comment))

I think that running tests with simple py.test command is better than python tests.py, but there are unused tests which will run that way. Solutions:

  • A. Get them away from pytest's sight with:
    1. Remove unused tests (test_sankey, test_skew, test_ttconv, and test_usetex from lib/matplotlib/tests)
    2. Rename test_only.py (to setup_test_only.py probably)
  • B. Make filter for collection phase.
    1. Implement whitelist
    2. Implement blacklist

I have tried both ways and they work, so what to choose is up to you.

  • py.test --collect-filter=none
  • py.test --collect-filter=blacklist
  • py.test --collect-filter=whitelist

Currently --collect-filter=whitelist is default (to follow same politics as nose does currently).

Image comparison decorator

Why do we have to rewrite image_comparison decorator? Because pytest-mpl plugin in the current state ("as is") is not usable for matplotlib:

  • + supports savefig_kwargs
  • + supports tolerance
  • ‒ need to supply baseline_dir in every decorator
  • ‒ does not support image format conversion
  • ‒ need something to do with freetype_version, remove_text, and style
@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jul 12, 2016
@story645
Copy link
Member

Do not inherit tests from unittest.UnitTest class

As someone who's a big fan of these, what's the rational? Unlike nose, it's part of the standard library and it doesn't seem like it's likely to be deprecated.

@QuLogic
Copy link
Member

QuLogic commented Jul 12, 2016

There are converters for nose and unittest, though they aren't totally perfect.

@story645
Copy link
Member

Yeah, I guess my question is "will pytest not run unittest" tests? Like I understand dumping nose 'cause it's deprecated, but I don't get the issue with unittest unless it explicitly won't work with pytest

@Kojoley
Copy link
Member Author

Kojoley commented Jul 12, 2016

They will work and they are working (pytest handles them with some internal loader hacking in _pytest.unitest). But it is useless import of a module that actuality does not used, and you can do same and even more more with pytest

@tacaswell
Copy link
Member

We would work with @astrofrog to get the features we need in pytest-mpl.

@jenshnielsen
Copy link
Member

@tacaswell are you sure we want to do that. It's the opposite of what @mdboom suggested in #5405

@astrofrog
Copy link
Contributor

astrofrog commented Jul 14, 2016

@tacaswell - I'd be happy to chat about this at SciPy!

@astrofrog
Copy link
Contributor

astrofrog commented Jul 14, 2016

Regarding the negatives for pytest-mpl:

‒ need to supply baseline_dir in every decorator

Actually this is not quite correct - there is a default value which is a folder called baseline relative to the test. I can also easily add a global setting.

‒ does not support image format conversion
‒ need something to do with freetype_version, remove_text, and style

I don't think there's any reason we can't add all this, and I don't think it should be that difficult. I'd love to work with you all to make pytest-mpl suitable for matplotlib! (these are all features I would find useful for other projects I use pytest-mpl for)

@QuLogic
Copy link
Member

QuLogic commented Oct 8, 2016

Another point: we need to get rid of the yield-based tests; they are causing tons of warning in the pytest built.

@QuLogic
Copy link
Member

QuLogic commented Nov 16, 2016

The yield-based tests are fixed, but do we still have a problem of not being able to use xdist?

@Kojoley
Copy link
Member Author

Kojoley commented Nov 16, 2016

The yield-based tests are fixed, but do we still have a problem of not being able to use xdist?

It looks like we can. At least I have tried and it is good. PR #7468 is opened.

@jnothman
Copy link

In my own project using pytest, attempting to use @image_comparison seems to make the test undetected by pytest. This seems to contradict https://github.com/matplotlib/matplotlib/blob/0d5a02a4e118eb098b78ea00039acd06310e54be/doc/devel/testing.rst. Is it known behaviour?

@dopplershift
Copy link
Contributor

Are you aware of https://github.com/matplotlib/pytest-mpl ?

@tacaswell
Copy link
Member

@jnothman This will be in the 2.1 release so unless you have installed from source you will not have it.

pytest-mpl is probably a better option.

@jnothman
Copy link

jnothman commented Mar 16, 2017 via email

@dopplershift
Copy link
Contributor

@jnothman Have you seen the remove_text option for image_comparison?

@phobson
Copy link
Member

phobson commented Mar 16, 2017

I think we can close this now. Is that the case?

@QuLogic
Copy link
Member

QuLogic commented Mar 16, 2017

There are still items unchecked that need to be implemented or decided to ignore.

@phobson
Copy link
Member

phobson commented Mar 16, 2017

Items I think we should ignore:

  • rewrite image_comparison decorator. (this is an enhancement and clearly not a prerequisite)
  • rewrite all assertions to pure assert. (i think numpy.testing.assert_* should stay as-is)

I'm -0 on using pytest-mock (like all testing libraries, its docks assume I already know way more than I do)

@timhoffm
Copy link
Member

timhoffm commented May 30, 2019

I've marked

Rewrite all assertations to pure assert

as done. I agree with @phobson that numpy.testing.* functions should be kept. All other asserts are already written.

Also checked

Rename test_only.py

because there is no such file anymore.

@anntzer
Copy link
Contributor

anntzer commented May 30, 2019

I decided to strikeout the "use pytest-mock" line (agreeing with @phobson above); unittest.mock works just fine in the rare cases where we use it and bringing in a new dependency seems overkill (unless someone has a good use case for it in the test suite, in which case please speak up :)).

@anntzer
Copy link
Contributor

anntzer commented May 30, 2019

The "rewrite image_comparison (currently it slows down collection due to baseline image copying)" item seems not true anymore:

  1. image copying only occurs in compare() now, not when collecting.
  2. Use symlinks instead of copies for test result_images. #10222 replaces copies by symlinks; at some point it gave a decent speedup but I can't repro it anymore at least locally.

@anntzer
Copy link
Contributor

anntzer commented Jul 4, 2019

Technically there is one setup_method left in test_transforms.py, but in that specific case it doesn't seem worthwhile to remove it.
I'm thus declaring this issue closed :) Thanks to all for worked on it!

@anntzer anntzer closed this as completed Jul 4, 2019
@story645 story645 removed this from the future releases milestone Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests