Skip to content

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

Merged
doutriaux1 merged 1 commit intomasterfrom
fix-checkimage
Apr 26, 2016
Merged

ENH #1854: Tests will check for alternate baselines using basename_[1…#1943
doutriaux1 merged 1 commit intomasterfrom
fix-checkimage

Conversation

@danlipsa
Copy link
Copy Markdown
Contributor

…-9].ext

Previously the pattern used was baseline.*.ext

@danlipsa
Copy link
Copy Markdown
Contributor Author

CDAT/uvcdat-testdata#127

@aashish24
Copy link
Copy Markdown
Contributor

LGTM 👍

…-9].ext

Previously the pattern used was baseline.*\.ext
@danlipsa
Copy link
Copy Markdown
Contributor Author

@doutriaux1 Please review.

@doutriaux1
Copy link
Copy Markdown
Contributor

@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
@doutriaux1 doutriaux1 deleted the fix-checkimage branch April 26, 2016 20:56
@danlipsa
Copy link
Copy Markdown
Contributor Author

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

agreed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants