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

ENH #1854: Tests will check for alternate baselines using basename_[1… #1943

Merged
merged 1 commit into from Apr 26, 2016

Conversation

Projects
None yet
3 participants
@danlipsa
Contributor

danlipsa commented Apr 21, 2016

…-9].ext

Previously the pattern used was baseline.*.ext

@danlipsa danlipsa force-pushed the fix-checkimage branch from 9f0dbbd to e9bc399 Apr 21, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 25, 2016

@danlipsa danlipsa force-pushed the fix-checkimage branch from e9bc399 to 6834146 Apr 25, 2016

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Apr 26, 2016

LGTM 👍

ENH #1854: Tests will check for alternate baselines using basename_[1…
…-9].ext

Previously the pattern used was baseline.*\.ext

@danlipsa danlipsa force-pushed the fix-checkimage branch from 6834146 to 0bc3f4d Apr 26, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 26, 2016

@doutriaux1 Please review.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 26, 2016

@danlipsa I'm cool with your changes, but in general I think it's unsafe to use alternate baselines.

First it adds work to maintaining the baselines, also I'm always worried a test gets better doesn't need the laternate baseline anymore, then we fix an issue with that test. A few week later a new change breaks the test but we don't see it because the old "bad" baseline is still here and passes.

@doutriaux1 doutriaux1 merged commit f281ff2 into master Apr 26, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@doutriaux1 doutriaux1 deleted the fix-checkimage branch Apr 26, 2016

@danlipsa

This comment has been minimized.

Contributor

danlipsa commented Apr 27, 2016

@doutriaux1 I am not happy with alternate baselines but I think they are a necessary evil. It boils down to the following choice: Should we spend a week to debug a small difference in how images are rendered on a specific platform, possibly narrow it down to a OpenGL difference in how the standard is interpreted or a OpenGL bug -- or spend a minute to add a new baseline. VTK uses alternate baselines. We run on many platforms/compilers. I think we only use 3 alternate baselines at most. For uvcdat I think we have at most one.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 27, 2016

agreed

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