Skip to content
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

Use some more pytest plugins: warnings & rerunfailures #8346

Merged
merged 7 commits into from
Apr 29, 2017

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Mar 20, 2017

Install pytest-warnings so we can see warnings produced by each test instead of being entirely hidden and unnoticed (cf. #7334 (comment)).

With pytest-rerunfailures, we can mark a few tests that are flaky so that they get rerun automatically. I'd rather flaky tests be fixed of course, but this will hopefully help speed up CI a bit since we won't have to wait the 12+ minutes for a full rebuild.

I have only marked two tests as flaky because they are inherently difficult to fix. If there are any suggestions for other tests to mark, I will add them, but hopefully there will be few.

@QuLogic QuLogic added this to the 2.1 (next point release) milestone Mar 20, 2017
@QuLogic
Copy link
Member Author

QuLogic commented Mar 20, 2017

With 3 test run failures out of the last 2 builds, test_mplot3d/mixedsubplot.svg might be a good addition as well. I actually have an idea of how to fix this one, but haven't had a chance to get through all the other required work to actually implement it.

appveyor.yml Outdated
@@ -93,6 +93,7 @@ install:
- cmd: IF %PYTHON_VERSION% == 2.7 conda install -q backports.functools_lru_cache
# pytest-cov>=2.3.1 due to https://github.com/pytest-dev/pytest-cov/issues/124
- conda install -q pytest "pytest-cov>=2.3.1" pytest-timeout pytest-xdist
- pip install -q pytest-rerunfailures pytest-warnings
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep everything installed from the same source?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not available in conda or conda-forge, but I could switch them all to pip.

Copy link
Contributor

Choose a reason for hiding this comment

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

let's use pip for everything

@phobson
Copy link
Member

phobson commented Mar 21, 2017

I was going to suggest the test_mixedsubplot-svg as well.

In my notes, I also have these:

  • lib/matplotlib/tests/test_mathtext.py::test_mathfont_stix_18
  • lib/matplotlib/tests/test_axes.py::test_formatter_ticker[1-formatter_ticker_002-pdf]

Though I haven't checked recently if they are still flaky

@QuLogic
Copy link
Member Author

QuLogic commented Mar 23, 2017

lib/matplotlib/tests/test_mathtext.py::test_mathfont_stix_18

This one's covered by c3aca56.

lib/matplotlib/tests/test_axes.py::test_formatter_ticker[1-formatter_ticker_002-pdf]

I don't recall seeing this one very often lately.

@anntzer anntzer changed the title Use some more pytest plugins: warnings & rerunfailures [MRG+1] Use some more pytest plugins: warnings & rerunfailures Mar 23, 2017
@QuLogic
Copy link
Member Author

QuLogic commented Mar 23, 2017

Hmm, you know what; I think the warnings are still not printed... I will investigate tomorrow.

@QuLogic
Copy link
Member Author

QuLogic commented Mar 23, 2017

Coincidentally, pytest-warnings just got merged into pytest: pytest-dev/pytest#2072

@dstansby
Copy link
Member

The doc build isn't finding the command deactivate, so is erroring - my guess is that this should be source deactivate?

@QuLogic
Copy link
Member Author

QuLogic commented Mar 26, 2017

Aha, I've figured out why warnings are not correctly captured. The image_comparison decorator runs the test code as part of the setup and only runs a compare in the actual test part, which means that it's not covered by pytest-warnings.

@QuLogic
Copy link
Member Author

QuLogic commented Mar 26, 2017

test_savefig_to_stringio_with_usetex has been a bit flaky as well; maybe should be added too?

Edit: Actually, that build is a lock error which I've not seen before. I do recall a test with a similar name (or just this one) failing semi-regularly though.

@@ -1,5 +1,7 @@
#! /bin/bash

set -e
Copy link
Member

Choose a reason for hiding this comment

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

This is causing the doc build to fail on line 39 below, which can just be deleted (I can't work out how to open a PR on your branch, but https://travis-ci.org/dstansby/matplotlib/builds/221288308 is my travis build)

Copy link
Member

Choose a reason for hiding this comment

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

should probably just drop line 39

@QuLogic
Copy link
Member Author

QuLogic commented Apr 15, 2017

FYI, I'd like to get #8380 in first so that the warning filter plugin starts working correctly.

@tacaswell
Copy link
Member

@QuLogic #8380 is in and this needs a rebase.

@QuLogic
Copy link
Member Author

QuLogic commented Apr 21, 2017

Well, that's unfortunate; it looks like mixedsubplots still fails even with a rerun. (Unfortunately, with xdist, you only see the last result, but I do know rerun is working as I made an explicitly broken test to try it out.) This is especially disappointing as mixedsubplots seems to have been failing quite a lot recently.

@phobson
Copy link
Member

phobson commented Apr 21, 2017

can we just disable the SVG portion of that test?

@QuLogic QuLogic force-pushed the pytest-plugins branch 2 times, most recently from b2528c5 to 76e4655 Compare April 23, 2017 18:53
@QuLogic
Copy link
Member Author

QuLogic commented Apr 23, 2017

I marked it as xfail now.

Since it depends on "speed", it may sometimes fail, so try again a
couple of times.
Even with a rerun, it doesn't appear to fix itself, and it's been acting
up a lot lately, so just ignore any failures for now.
@QuLogic
Copy link
Member Author

QuLogic commented Apr 28, 2017

This is rebased now that the documentation is building again.

@WeatherGod
Copy link
Member

As the creator of mixedsubplots, I am completely flummoxed why it would even be flakey... ever. I worry that there might actually be a real bug somewhere here. A race condition or something, perhaps.

@QuLogic
Copy link
Member Author

QuLogic commented Apr 28, 2017

Not sure it's a race condition as marking it to rerun a few times did not seem to work. Once it's broken, it's broken, but I don't know what breaks it.

@WeatherGod
Copy link
Member

Could you try taking out the antialiased argument in the plot_surface(). It is an anachronism from a bygone era. Perhaps it is causing trouble.

@QuLogic
Copy link
Member Author

QuLogic commented Apr 28, 2017

According to #8549, that didn't work, so I think we should just go ahead and merge this.

@phobson phobson changed the title [MRG+1] Use some more pytest plugins: warnings & rerunfailures Use some more pytest plugins: warnings & rerunfailures Apr 29, 2017
@phobson phobson merged commit 405e595 into matplotlib:master Apr 29, 2017
@QuLogic QuLogic deleted the pytest-plugins branch April 30, 2017 01:24
@QuLogic
Copy link
Member Author

QuLogic commented Apr 30, 2017

🎊 Hopefully there's at least a little reduction in restarted jobs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants