Re: [PATCH v4 2/5] t: move reftable/tree_test.c to the unit testing framework
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jul 17, 2024, 12:39 UTC
- Message-ID
- <CAOLa=ZQxDXDDWXQrt9kpykuMr6nxSA8uf2U2nu0ChTf3yuH8sQ@mail.gmail.com>
- In-Reply-To
- <20240716075641.4264-3-chandrapratap3519@gmail.com>
Chandra Pratap <chandrapratap3519@gmail.com> writes:
Show 6 quoted lines
> reftable/tree_test.c exercises the functions defined in
> reftable/tree.{c, h}. Migrate reftable/tree_test.c to the unit
> testing framework. Migration involves refactoring the tests to use
> the unit testing framework instead of reftable's test framework and
> renaming the tests to align with unit-tests' standards.
>On second thought, it's easier for me to review here for the existing state of the code. So let me do that..
[snip]
Show 22 quoted lines
> diff --git a/t/unit-tests/t-reftable-tree.c b/t/unit-tests/t-reftable-tree.c
> new file mode 100644
> index 0000000000..5df814d983
> --- /dev/null
> +++ b/t/unit-tests/t-reftable-tree.c
> @@ -0,0 +1,56 @@
> +/*
> +Copyright 2020 Google LLC
> +
> +Use of this source code is governed by a BSD-style
> +license that can be found in the LICENSE file or at
> +https://developers.google.com/open-source/licenses/bsd
> +*/
> +
> +#include "test-lib.h"
> +#include "reftable/tree.h"
> +
> +static int t_compare(const void *a, const void *b)
> +{
> + return (char *)a - (char *)b;
> +}
> +So this is the comparison code, and we're expecting the values to be a character. Okay.
Show 16 quoted lines
> +struct curry {
> + void *last;
> +};
> +
> +static void check_increasing(void *arg, void *key)
> +{
> + struct curry *c = arg;
> + if (c->last)
> + check_int(t_compare(c->last, key), <, 0);
> + c->last = key;
> +}
> +
> +static void t_tree(void)
> +{
> + struct tree_node *root = NULL;
> + void *values[11] = { 0 };Although we were comparing 'char' above, here we have a 'void *' array. Why?
Show 7 quoted lines
> + struct tree_node *nodes[11] = { 0 };
> + size_t i = 1;
> + struct curry c = { 0 };
> +
> + do {
> + nodes[i] = tree_search(values + i, &root, &t_compare, 1);
> + i = (i * 7) % 11;It gets weirder, we calculate 'i' as {7, 5, 2, 3, 10, 4, 6, 9, 8, 1}. We use that to index 'values', but values is '0' initialized, so we always send '0' to tree_search? Doesn't that make this whole thing a moot? Or did I miss something?
Show 19 quoted lines
> + } while (i != 1);
> +
> + for (i = 1; i < ARRAY_SIZE(nodes); i++) {
> + check_pointer_eq(values + i, nodes[i]->key);
> + check_pointer_eq(nodes[i], tree_search(values + i, &root, &t_compare, 0));
> + }
> +
> + infix_walk(root, check_increasing, &c);
> + tree_free(root);
> +}
> +
> +int cmd_main(int argc, const char *argv[])
> +{
> + TEST(t_tree(), "tree_search and infix_walk work");
> +
> + return test_done();
> +}
> --
> 2.45.GIT