Skip to content

Fixed the ordering of images for regression testing#1307

Merged
doutriaux1 merged 1 commit intoreleasefrom
fix_vcs3d_test_image_order
May 11, 2015
Merged

Fixed the ordering of images for regression testing#1307
doutriaux1 merged 1 commit intoreleasefrom
fix_vcs3d_test_image_order

Conversation

@aashish24
Copy link
Copy Markdown
Contributor

So that the cdash will show them correctly in the right
order

@jbeezley @doutriaux1 @ThomasMaxwell please review

So that the cdash will show them correctly in the right
order
@doutriaux1
Copy link
Copy Markdown
Contributor

Thanks!

doutriaux1 added a commit that referenced this pull request May 11, 2015
Fixed the ordering of images for regression testing
@doutriaux1 doutriaux1 merged commit 66bab1b into release May 11, 2015
@jbeezley
Copy link
Copy Markdown
Contributor

That caused the error to go away completely when it should have just switched the images in cdash. This line makes me very suspicious. I think an IOError is raised in the try block somewhere causing the test to return successfully when it should actually fail. If there really is a reason to do this, the condition should probably be more narrowly defined.

@doutriaux1
Copy link
Copy Markdown
Contributor

what are you saying @jbeezley ? You don't like "magic"?
More seriously should we worry about this now, or open a new issue for 2.3? I'll leave it up to you.

@jbeezley
Copy link
Copy Markdown
Contributor

I would say leave it for 2.3 and go over all of the tests to make sure failures are reported correctly (as in #1015).

@aashish24 aashish24 deleted the fix_vcs3d_test_image_order branch May 12, 2015 13:22
@aashish24
Copy link
Copy Markdown
Contributor Author

@jbeezley agree with you. it should have ret = (anything but 0). I will push a branch post 2.2 release.

@aashish24 aashish24 mentioned this pull request May 22, 2015
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