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

globally (effectively) --catch_system_errors=no all boost-test based unit tests #1849

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

spoonincode
Copy link
Member

Some of our tests require --catch_system_errors=no to properly operate, but there is another compelling reason to blanket this option on all tests: gathering core dumps. Default behavior (--catch_system_errors=yes) will block core dump creation for those tests should they fail. Instead of needing to remember to sprinkle --catch_system_errors=no in all the various add_test() declarations (prone to missing some), we have some other options,

  • set BOOST_TEST_CATCH_SYSTEM_ERRORS environment variable in CI. This allows CI to gobble up core dumps from any test, but introduces a difference in CI vs non-CI behavior
  • plumb the option in boost test's C++ initialization (but this requires manual plumbing for every test)
  • set the BOOST_TEST_DEFAULTS_TO_CORE_DUMP preprocessor macro
    • Secondary option: plumb through a new INTERFACE library that depends on Boost::unit_test_framework to add BOOST_TEST_DEFAULTS_TO_CORE_DUMP; but this still requires remembering in every place we use boost test we need to link to leap_boost_unit_test instead of Boost::unit_test_framework

As much as I detest "global" cmake compile definitions, the BOOST_TEST_DEFAULTS_TO_CORE_DUMP preprocessor macro seems the best option in terms of being a single location with consistent blanket behavior. Certainly open to opinions though.

Work on #640

Copy link
Member

@heifner heifner 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 always considered default behavior of --catch_system_errors=yes to be an anti-feature.

@spoonincode spoonincode merged commit 14544ec into main Nov 1, 2023
29 checks passed
@spoonincode spoonincode deleted the cse=no_5x branch November 1, 2023 20:57
@spoonincode spoonincode changed the title [5.0] globally (effectively) --catch_system_errors=no all boost-test based unit tests globally (effectively) --catch_system_errors=no all boost-test based unit tests Nov 1, 2023
@spoonincode spoonincode linked an issue Nov 1, 2023 that may be closed by this pull request
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.

CICD: Capture core dump files from CI runs
3 participants