Add valgrind to CI

[saghul]

Fixes: #71

[saghul]

1h 32m 😅

Do we think that is acceptable? @bnoordhuis @chqrlie

[chqrlie]

1h 32m 😅

Do we think that is acceptable? @bnoordhuis @chqrlie

I would say no, this makes the CI too long for every patch request. Can we make this test optional and trigger it manually?

[saghul]

Can we make this test optional and trigger it manually?

Yes, it's possible to make a dedicated workflow that requires manual dispatch.

[bnoordhuis]

Or, alternatively, run it post-merge when things get committed to the master branch. That way it doesn't hold up development.

There are ways to speed up valgrind a little, like e.g. setting --num-callers=5, but it probably won't make a huge difference.

And, on the flip side, --track-origins=yes would definitely be useful (tells you not just that you have an uninitialized value but also where it comes from) but makes valgrind at least twice as slow.

[saghul]

Or, alternatively, run it post-merge when things get committed to the master branch. That way it doesn't hold up development.

I like that. I'll make the change.

And, on the flip side, --track-origins=yes would definitely be useful (tells you not just that you have an uninitialized value but also where it comes from) but makes valgrind at least twice as slow.

Let's have it!

[saghul]

Updated as per @bnoordhuis 's suggestions. I also enabled manual dispatch of the workflow, in case we want to run it without pushing to master, for whatever reason.