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

Don't skip asserts in unit test release builds #1236

Conversation

rogernelson
Copy link
Collaborator

Fixes #1235

Summarize your change.

This branch undefines NDEBUG when using unit tests. This is to ensure the asserts used in assertEqual and assertNotEqual, etc fail the test even in release builds. Since this file is only included by the unit tests it won't have any impact outside of that scope.

Reference associated tests.

test_clip.cpp and test_opentime.cpp

@rogernelson
Copy link
Collaborator Author

I can't modify the list of reviewers, but I'd like to request that @darbyjohnston review this.

@darbyjohnston
Copy link
Contributor

Seems like a reasonable change, I guess I had assumed the tests would always be run in Debug mode.

@meshula
Copy link
Collaborator

meshula commented Mar 9, 2022

From a philosophical standpoint, tests need to be run in Release, otherwise they don't reflect the common deployment situation. It's good to also run in Debug, although one sees that less often in the wild.

Copy link
Collaborator

@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.

Thanks for the catch! Over time we should move away from cstd assertions, and towards a framework of some sort. If anyone is thinking about that, lighter is better, speaking for myself, I'd prefer a homegrown framework, or at least a framework that is not larger than OTIO itself ;)

@meshula meshula merged commit 46d19ef into AcademySoftwareFoundation:main Mar 9, 2022
@ssteinbach ssteinbach added this to the Public Beta 15 milestone Sep 19, 2022
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.

C++ Unit tests always pass in release builds
4 participants