Hi Taylor
On 31/05/2023 23:35, Taylor Blau wrote:
Hi Siddharth,
On Wed, May 31, 2023 at 05:51:41PM +0200, Siddharth Singh wrote:
This RFC patch adds unit tests for khash.h. It uses the C TAP harness
to illustrate the test cases [1]. This is not intended to be a
complete implementation. The purpose of this patch to get your
thoughts on the unit test content, not the test framework itself.
Thanks for working on this, and for opening the discussion up. I took
only a brief look through the actual changes. But I think the much more
interesting discussion is on the approach, so I'll refrain from
commenting on the tests themselves.
I am somewhat skeptical of this as a productive direction, primarily
because khash already has tests [1] that exercise its functionality. I'm
not necessarily opposed to (light) testing of oid_set, oid_map, and
oid_pos, which are khash structures, but declared by Git.
I agree that starting by adding tests for third party code is a bit
curious, but those tests you linked to look like they're testing
performance rather than correctness.
Best Wishes
Phillip
Even still, I don't think that we are testing much in that case, since
the boundary between Git and khash is limited to the KHASH_INIT macro.
So... I dunno. I'm not strongly opposed here, but I think this is
probably not the most productive place to start adding tests.
Thanks,
Taylor
[1]: https://github.com/attractivechaos/klib/blob/master/test/khash_test.c
