valgrind checks in CI

Question

valgrind checks in CI

tisaac opened this issue 3 years ago · 9 comments

tisaac commented 3 years ago

Description

Memory leaks can be integrated into releases and development branches silently right now

Proposed solution

Add a CI runner that configures the test environment to run with valgrind.

Answer 1 · 2022-08-18T13:04:00.000Z

Would you want to add a PR to this and/or coordinate with @gghosh8495 working on our CI?

Answer 2 · 2022-09-28T19:44:33.000Z

It's proposed on branch feature-test-valgrind. I'd be happpy for comments.

Answer 3 · 2022-09-28T20:07:29.000Z

The CI takes a really long time then. Thoughts?

Answer 4 · 2022-09-28T20:13:16.000Z

We could take the valgrind CI out again and you may run configure --enable-valgrind on your end as needed?

Answer 5 · 2022-09-29T11:24:00.000Z

Now removing debug mode and optimizing with -O2 brings the runtime down to under 20 minutes.
This may be sufficient for github. Anybody is free to test their configuration using --enable-valgrind.

Answer 6 · 2022-09-29T17:08:19.000Z

@cburstedde The valgrind test for CMake is trivial. CTest has in-built option to run valgrind in tests. I will update the documentation.

Answer 7 · 2022-09-29T17:12:47.000Z

@cburstedde The valgrind test for CMake is trivial. CTest has in-built option to run valgrind in tests. I will update the documentation.

Thanks. Would it make sense to add such a job to ci_valgrind.yml?

Answer 8 · 2022-09-29T17:14:32.000Z

@cburstedde Yes, I will update the file.

Answer 9 · 2022-10-29T13:10:20.000Z

So we have CI valgrind testing now.
To keep run times acceptable, we are using -O2 on the valgrind compile.
Occasionally we see p8est_is_balanced throw a valgrind error, and we have eliminated p8est_test_balanced from the valgrind CI run. This would need to be investigated.