On Wed, Jun 12, 2024 at 11:08:14AM +0530, Chandra Pratap wrote:
> In the current testing setup for infix_walk(), the following
> properties of an infix traversal of a tree remain untested:
> - every node of the tree must be visited
> - every node must be visited exactly only
s/only/once
> and only the property 'traversal in increasing order' is tested.
Nit: this reads a bit awkward. How about "In fact, we only verify that
the traversal happens in increasing order."
> @@ -51,6 +50,7 @@ static void test_infix_walk(void)
> {
> struct tree_node *root = NULL;
> void *values[11] = { 0 };
> + void *out[20] = { 0 };
From the test below it looks like we only expect 11 values to be added
to `out`. Why does this array have length 20?
We could of course also use something like `ALLOC_GROW()` to grow the
array dynamically. But that'd likely be overkill.
> struct curry c = { 0 };
> size_t i = 1;
>
> @@ -59,7 +59,11 @@ static void test_infix_walk(void)
> i = (i * 7) % 11;
> } while (i != 1);
>
> - infix_walk(root, &check_increasing, &c);
> + c.arr = (void **) &out;
We can initialize this variable directly when declaring `c`:
struct curry c = {
.arr = &out;
};
Also, is the cast necessary? This is the only site where we use `struct
curry` if I'm not mistaken, so I'd expect that the type of `arr` should
match our expectations.
> + infix_walk(root, &store, &c);
> + for (i = 1; i < ARRAY_SIZE(values); i++)
> + check_pointer_eq(values + i, out[i - 1]);
Let's also verify that `c.len` matches the expected number of nodes
visited.
Patrick
> + check(!out[i]);
> tree_free(root);
> }
Attachment:
signature.asc
Description: PGP signature
