Skip to content
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

reduce iterations in testIDManifest to speed up #1180

Merged

Conversation

peterhillman
Copy link
Contributor

testIDManifest was timing out on slow processors due to 1500 second test limit.
This change runs 20 different random manifests instead of 100 to speed it up. On Raspberry Pi with ARM7 architecture, testIDManifest now completes in around 500 seconds. (Slowest test is now testHuf, at just over 1000s)

Signed-off-by: Peter Hillman peterh@wetafx.co.nz

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

I've never liked this test. If the random seed is always the same, it's not really random, it'll be the same sequence of random entries. Testing the same set of 1000 words year after year really doesn't feel like it's telling us anything at all, except that the sequence is fine. (We are reseeding an LCG generator, std::default_random_generator, with the same value). It seems to me like we should be deliberately checking a small number of edge cases and deliberately constructed "could fail" tests. Otherwise what's the point of this long running test?

Short of rewriting this test, shortening the test sure gets my vote.

@cary-ilm cary-ilm merged commit daef853 into AcademySoftwareFoundation:master Oct 21, 2021
cary-ilm pushed a commit to cary-ilm/openexr that referenced this pull request Oct 21, 2021
cary-ilm pushed a commit that referenced this pull request Oct 24, 2021
Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants