Skip to content

Use some more pytest plugins: warnings & rerunfailures - #8346

Merged
phobson merged 7 commits into
matplotlib:masterfrom
QuLogic:pytest-plugins
Apr 29, 2017
Merged

phobson merged 7 commits into
matplotlib:masterfrom
QuLogic:pytest-plugins

Conversation

@QuLogic

@QuLogic QuLogic commented Mar 20, 2017

Copy link
Copy Markdown
Member

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

QuLogic commented Mar 20, 2017 •

Copy link
Copy Markdown
Member Author

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.

Comment thread appveyor.yml Outdated
- 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

phobson commented Mar 21, 2017

Copy link
Copy Markdown
Member

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

QuLogic commented Mar 23, 2017

Copy link
Copy Markdown
Member Author

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

QuLogic commented Mar 23, 2017

Copy link
Copy Markdown
Member Author

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

@QuLogic

QuLogic commented Mar 23, 2017

Copy link
Copy Markdown
Member Author

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

@dstansby

Copy link
Copy Markdown
Member

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

@QuLogic

QuLogic commented Mar 26, 2017

Copy link
Copy Markdown
Member Author

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

QuLogic commented Mar 26, 2017 •

Copy link
Copy Markdown
Member Author

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.

Comment thread ci/travis/test_script.sh
@@ -1,5 +1,7 @@
#! /bin/bash

set -e

Copy link
Copy Markdown
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
Copy Markdown
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

QuLogic commented Apr 15, 2017

Copy link
Copy Markdown
Member Author

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

@tacaswell

Copy link
Copy Markdown
Member

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

@QuLogic

QuLogic commented Apr 21, 2017

Copy link
Copy Markdown
Member Author

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

phobson commented Apr 21, 2017

Copy link
Copy Markdown
Member

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

QuLogic commented Apr 23, 2017

Copy link
Copy Markdown
Member Author

I marked it as xfail now.

QuLogic added 2 commits April 27, 2017 23:11
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

QuLogic commented Apr 28, 2017

Copy link
Copy Markdown
Member Author

This is rebased now that the documentation is building again.

@WeatherGod

Copy link
Copy Markdown
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

QuLogic commented Apr 28, 2017

Copy link
Copy Markdown
Member Author

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
Copy Markdown
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.

@phobson phobson mentioned this pull request Apr 28, 2017
2 tasks done
@QuLogic

QuLogic commented Apr 28, 2017

Copy link
Copy Markdown
Member Author

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

QuLogic commented Apr 30, 2017

Copy link
Copy Markdown
Member Author

🎊 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