X Tutup
Skip to content

TST: fail on missing baseline file#8262

Merged
QuLogic merged 1 commit intomatplotlib:masterfrom
tacaswell:tst_fail_on_missing_baseline
Mar 13, 2017
Merged

TST: fail on missing baseline file#8262
QuLogic merged 1 commit intomatplotlib:masterfrom
tacaswell:tst_fail_on_missing_baseline

Conversation

@tacaswell
Copy link
Copy Markdown
Member

The change from a fail to knownfail on missing file come in via 3f3991a which I apparently did not review carefully enough, sorry.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Mar 10, 2017
@tacaswell tacaswell requested review from Kojoley and anntzer March 10, 2017 21:21
@anntzer
Copy link
Copy Markdown
Contributor

anntzer commented Mar 10, 2017

You probably meant to raise that exception. But I wonder whether we even need to bother doing the copy -- why can't we just compare with the image in its original folder (and fail on access if the image is not present)?

@tacaswell tacaswell force-pushed the tst_fail_on_missing_baseline branch from b5e777e to a4c309e Compare March 10, 2017 21:33
@tacaswell
Copy link
Copy Markdown
Member Author

Force-pushed a fix to raise.

That is an interesting question, the copying goes back to a5729a5 with the comment "testing bugfix: don't save test results to site-packages".

One nice thing about copying the files is that it puts them (along with the results) next to each other with slightly different names which is something that tools/test_triage.py relies on, but it probably would give a nice speed up (and we can fix test_triage with out too much trouble).

Copy link
Copy Markdown
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

LGTM as a quickfix, but as mentioned in the thread I think some additional refactoring is of order.

@QuLogic QuLogic merged commit c6aebde into matplotlib:master Mar 13, 2017
@tacaswell tacaswell deleted the tst_fail_on_missing_baseline branch March 13, 2017 20:16
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.

3 participants

X Tutup