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 sanitizer build options #249

Merged
merged 6 commits into from
Jan 20, 2022
Merged

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Dec 1, 2021

BEGINRELEASENOTES

  • Add a USE_SANITIZER build option to more easily build podio with sanitizers for testing. Curently Address, Memory[WithOrigin], Undefined and Thread are available as options. Given the limitations of the sanitizers these are more or less mutually exlusive.
  • Label all the Catch2 test cases which makes it easier to run them selectively.
  • For builds with sanitizers enabled, by default ignore tests with known failures, but add a FORCE_RUN_ALL_TESTS cmake option that overrides this for local development.
  • Run CI workflows with a selection of sanitizers enabled (on a limited list of tests).

ENDRELEASENOTES

cherry-picked from #188 (also present in #220)

@tmadlener tmadlener force-pushed the add-sanitizer-builds branch 2 times, most recently from d3fc211 to d075d59 Compare December 3, 2021 10:34
@tmadlener
Copy link
Collaborator Author

I put in a few more things into this PR, because I got annoyed that I couldn't run at least the tests that run successfully at the moment with sanitizers enabled. I will open an issue to keep track of the known test failures, so that we can start to hopefully enable more and more tests even in the sanitizer builds. Currently (almost) all of the "integration style" tests fail with any of the sanitizers, so they will all be ignored when run via ctest. For the Catch2 unittests, the situation is a bit more diverse, and I have introduced the [ASAN-FAIL], [LEAK-FAIL] and [THREAD-FAIL] tags to mark (and subsequently ignore) the tests which are currently failing. Most of these failures are related to #174 (I think).

In principle this now also makes it possible to add failing tests for issues with a reproducer and ignore them by default, but still be able to easily run them locally.

@hegner
Copy link
Collaborator

hegner commented Jan 20, 2022

please document the additions. thanks.

- Add a list of possible sanitizers that can be used to build the whole
  of podio with the USE_SANITIZER option
- At the moment only build podio but do not run the tests, since there
  are a few things that need to be fixed before they can be succesfully
  run with them
- Add a workflow file that builds podio with clang and gcc with all
  available sanitizers for the respective toolchain
- Additionally make sure that we either use auto test discovery or run
  the complete executable, not both
- Add an option to still run all the tests locally on demand
@hegner hegner merged commit 18f67fb into AIDASoft:master Jan 20, 2022
@tmadlener tmadlener deleted the add-sanitizer-builds branch January 20, 2022 15:59
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