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

Add tests for bin programs #1351

Merged
merged 4 commits into from
Mar 11, 2023
Merged

Conversation

cary-ilm
Copy link
Member

@cary-ilm cary-ilm commented Mar 8, 2023

Python-based tests for:

  • exr2aces
  • exrcheck
  • exrenvmap
  • exrheader
  • exrinfo
  • exrmakepreview
  • exrmaketiled
  • exrmultipart
  • exrmultiview
  • exrstdattr

Also:

  • The tests require Python3 and are skipped if it's not available
  • The tests download test images at build time from https://github.com/AcademySoftwareFoundation/openexr-images
  • Tests generally confirm the significant bits of output printed to stdout, so rely on the formatting of exrinfo
  • All programs capitialize "Usage" in their usage message
  • exrmultipart now prints its usage message to stderr rather than cout
  • Fixed an uninitialized variable in exrinfo.c that caused inconsistent return code

Python-based tests for:
- exr2aces
- exrcheck
- exrenvmap
- exrheader
- exrinfo
- exrmakepreview
- exrmaketiled
- exrmultipart
- exrmultiview
- exrstdattr

Also:
- The tests require Python3 and are skipped if it's not available
- The tests download test images at build time from
  https://github.com/AcademySoftwareFoundation/openexr-images
- All programs capitialize "Usage" in their usage message
- exrmultipart now prints its usage message to stderr rather than cout

Signed-off-by: Cary Phillips <cary@ilm.com>
@meshula
Copy link
Contributor

meshula commented Mar 9, 2023

This is great to see. Is it worth opening an issues page or a Google Sheet with some sort of chart or matrix of "test thoroughness"? e.g. the cubemap test is currently superficial, so it could have some suggested tests to add like "does the output contain a cube map?" "are all mip levels present?" That could turn into a Summer of Code type proposal in the end.

@cary-ilm
Copy link
Member Author

cary-ilm commented Mar 9, 2023

The envmap was the last one I did, and I ran out of steam :(. I'll beef it up. But yes, good idea, I'll also add issues for the areas of code that still lack coverage, there are a few.

@lgritz
Copy link
Contributor

lgritz commented Mar 9, 2023

In general, I find it easier to generate the output once, check that in as a reference output, and then the test is just verifying that you match that output, rather than these kinds of line-by-line comparisons.

@cary-ilm
Copy link
Member Author

cary-ilm commented Mar 9, 2023

I thought about that, but I worried that a direct capture of the output might be too fragile, since the output of "exrinfo -v" comes from looping over the internal list of attributes, if something were to change the order of that list, the order of the output might change. I'm not sure that would ever actually happen, but it didn't seem to hard to just cherry-pick the significant bits of output.

Signed-off-by: Cary Phillips <cary@ilm.com>
@meshula
Copy link
Contributor

meshula commented Mar 10, 2023

I wasn't actually meaning to prompt you to write the test for the cubemap, I was just using it as an example where we could request assistance, sorry for the extra work!

And reference a tag on the openexr-images repo, for future-proofing

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
@cary-ilm
Copy link
Member Author

I intended to fill out the envmap test anyway!

@cary-ilm cary-ilm merged commit 1d4366a into AcademySoftwareFoundation:main Mar 11, 2023
@cary-ilm cary-ilm added the v3.2.0 label Jul 9, 2023
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.

None yet

3 participants