diff mbox series

unit-tests: convert t/helper/test-oid-array.c to unit-tests

Message ID 20240223193257.9222-1-shyamthakkar001@gmail.com (mailing list archive)
State New
Headers show
Series unit-tests: convert t/helper/test-oid-array.c to unit-tests | expand

Commit Message

Ghanshyam Thakkar Feb. 23, 2024, 7:32 p.m. UTC
Migrate t/helper/test-oid-array.c and t/t0064-oid-array.sh to the
recently added unit testing framework. This would improve runtime
performance and provide better debugging via displaying array content,
index at which the test failed etc. directly to stdout.

There is only one change in the new testing approach. In the previous
testing method, a new repo gets initialized for the test according to
GIT_TEST_DEFAULT_HASH algorithm. In unit testing however, we do not
need to initialize the repo. We can set the length of the hexadecimal
strbuf according to the algorithm used directly.

Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
[RFC]: I recently saw a series by Eric W. Biederman [1] which enables
the use of oid's with different hash algorithms into the same
oid_array safely. However, there were no tests added for this. So, I
am wondering if we should have a input format which allows us to
specify hash algo for each oid with its hex value. i.e. "sha1:55" or
"sha256:55", instead of just "55" and relying on GIT_TEST_DEFAULT_HASH
for algo. So far, I tried to imitate the existing tests but I suppose
this may be useful in the future if that series gets merged.

 Makefile                   |   2 +-
 t/helper/test-oid-array.c  |  45 --------
 t/helper/test-tool.c       |   1 -
 t/helper/test-tool.h       |   1 -
 t/t0064-oid-array.sh       | 104 -----------------
 t/unit-tests/t-oid-array.c | 222 +++++++++++++++++++++++++++++++++++++
 6 files changed, 223 insertions(+), 152 deletions(-)
 delete mode 100644 t/helper/test-oid-array.c
 delete mode 100755 t/t0064-oid-array.sh
 create mode 100644 t/unit-tests/t-oid-array.c

Comments

Ghanshyam Thakkar Feb. 23, 2024, 7:37 p.m. UTC | #1
On Sat Feb 24, 2024 at 1:02 AM IST, Ghanshyam Thakkar wrote:
> [RFC]: I recently saw a series by Eric W. Biederman [1] which enables
> the use of oid's with different hash algorithms into the same
> oid_array safely. However, there were no tests added for this. So, I
> am wondering if we should have a input format which allows us to
> specify hash algo for each oid with its hex value. i.e. "sha1:55" or
> "sha256:55", instead of just "55" and relying on GIT_TEST_DEFAULT_HASH
> for algo. So far, I tried to imitate the existing tests but I suppose
> this may be useful in the future if that series gets merged.

[1]:
https://lore.kernel.org/git/20231002024034.2611-2-ebiederm@gmail.com/

apologies for the noise.
Christian Couder Feb. 26, 2024, 3:11 p.m. UTC | #2
On Fri, Feb 23, 2024 at 8:33 PM Ghanshyam Thakkar
<shyamthakkar001@gmail.com> wrote:
>
> Migrate t/helper/test-oid-array.c and t/t0064-oid-array.sh to the
> recently added unit testing framework. This would improve runtime
> performance and provide better debugging via displaying array content,
> index at which the test failed etc. directly to stdout.

It might not be a good idea to start working on a GSoC project we
propose (Move existing tests to a unit testing framework) right now.
You can work on it as part of your GSoC application, to show an
example of how you would do it, and we might review that as part of
reviewing your application. But for such a project if many candidates
started working on it and sent patches to the mailing list before they
get selected, then the project might be nearly finished before the
GSoC even starts.

So I think it would be better to work on other things instead, like
perhaps reviewing other people's work or working on other bug fixes or
features. Anyway now that this is on the mailing list, I might as well
review it as it could help with your application. But please consider
working on other things.

> There is only one change in the new testing approach. In the previous
> testing method, a new repo gets initialized for the test according to
> GIT_TEST_DEFAULT_HASH algorithm.

It looks like this happens in "t/test-lib-functions.sh", right?
Telling a bit more about how and where that happens might help
reviewers who would like to take a look.

> In unit testing however, we do not
> need to initialize the repo. We can set the length of the hexadecimal
> strbuf according to the algorithm used directly.

So is your patch doing that or not? It might be better to be explicit.
Also if 'strbuf's are used, then is it really worth it to set their
length in advance, instead of just letting them grow to the right
length as we add hex to them?

> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> ---
> [RFC]: I recently saw a series by Eric W. Biederman [1] which enables
> the use of oid's with different hash algorithms into the same
> oid_array safely. However, there were no tests added for this. So, I
> am wondering if we should have a input format which allows us to
> specify hash algo for each oid with its hex value. i.e. "sha1:55" or
> "sha256:55", instead of just "55" and relying on GIT_TEST_DEFAULT_HASH
> for algo. So far, I tried to imitate the existing tests but I suppose
> this may be useful in the future if that series gets merged.

The fact that there is a series touching the same area might also hint
that it might not be the right time to work on this.

> diff --git a/t/unit-tests/t-oid-array.c b/t/unit-tests/t-oid-array.c
> new file mode 100644
> index 0000000000..b4f43c025d
> --- /dev/null
> +++ b/t/unit-tests/t-oid-array.c
> @@ -0,0 +1,222 @@
> +#include "test-lib.h"
> +#include "hex.h"
> +#include "oid-array.h"
> +#include "strbuf.h"
> +
> +#define INPUT "88", "44", "aa", "55"
> +#define INPUT_DUP \
> +       "88", "44", "aa", "55", "88", "44", "aa", "55", "88", "44", "aa", "55"

Can you reuse INPUT in INPUT_DUP?

> +#define INPUT_ONLY_DUP "55", "55"
> +#define ENUMERATION_RESULT_SORTED "44", "55", "88", "aa"
> +
> +/*
> + * allocates the memory based on the hash algorithm used and sets the length to
> + * it.
> + */
> +static void hex_strbuf_init(struct strbuf *hex)
> +{
> +       static int sz = -1;
> +
> +       if (sz == -1) {
> +               char *algo_env = getenv("GIT_TEST_DEFAULT_HASH");
> +               if (algo_env && !strcmp(algo_env, "sha256"))
> +                       sz = GIT_SHA256_HEXSZ;
> +               else
> +                       sz = GIT_SHA1_HEXSZ;
> +       }
> +
> +       strbuf_init(hex, sz);
> +       strbuf_setlen(hex, sz);
> +}

A strbuf can grow when we add stuff to it. We don't need to know its
size in advance. So I am not sure this function is actually useful.

> +/* fills the hex strbuf with alternating characters from 'c' */
> +static void fill_hex_strbuf(struct strbuf *hex, char *c)
> +{
> +       size_t i;
> +       for (i = 0; i < hex->len; i++)
> +               hex->buf[i] = (i & 1) ? c[1] : c[0];

There is strbuf_addch() to add a single char to a strbuf, or
strbuf_add() and strbuf_addstr() to add many chars at once.

> +}
Ghanshyam Thakkar Feb. 26, 2024, 7:11 p.m. UTC | #3
On Mon Feb 26, 2024 at 8:41 PM IST, Christian Couder wrote:
> It might not be a good idea to start working on a GSoC project we
> propose (Move existing tests to a unit testing framework) right now.
> You can work on it as part of your GSoC application, to show an
> example of how you would do it, and we might review that as part of
> reviewing your application. But for such a project if many candidates
> started working on it and sent patches to the mailing list before they
> get selected, then the project might be nearly finished before the
> GSoC even starts.
>
> So I think it would be better to work on other things instead, like
> perhaps reviewing other people's work or working on other bug fixes or
> features. Anyway now that this is on the mailing list, I might as well
> review it as it could help with your application. But please consider
> working on other things.

I understand and will work on other things.

> > There is only one change in the new testing approach. In the previous
> > testing method, a new repo gets initialized for the test according to
> > GIT_TEST_DEFAULT_HASH algorithm.
>
> It looks like this happens in "t/test-lib-functions.sh", right?
> Telling a bit more about how and where that happens might help
> reviewers who would like to take a look.

Yeah, that is happens in "t/test-lib-functions.sh". I will update the
commit message to describe this better.
>
> > In unit testing however, we do not
> > need to initialize the repo. We can set the length of the hexadecimal
> > strbuf according to the algorithm used directly.
>
> So is your patch doing that or not? It might be better to be explicit.
> Also if 'strbuf's are used, then is it really worth it to set their
> length in advance, instead of just letting them grow to the right
> length as we add hex to them?

I thought of it like this: If we were to just let them grow, then we
would need separate logic for reusing that strbuf or use a different
one everytime since it always grows. By separating allocation
(hex_strbuf_init) and manipulation (fill_hex_strbuf), that same strbuf
can be reused for different hex values.

But, none of the test currently need to reuse the same strbuf, so I
suppose it is better to just let it grow and even if the need arises we
can use strbuf_splice().


> > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> > ---
> > [RFC]: I recently saw a series by Eric W. Biederman [1] which enables
> > the use of oid's with different hash algorithms into the same
> > oid_array safely. However, there were no tests added for this. So, I
> > am wondering if we should have a input format which allows us to
> > specify hash algo for each oid with its hex value. i.e. "sha1:55" or
> > "sha256:55", instead of just "55" and relying on GIT_TEST_DEFAULT_HASH
> > for algo. So far, I tried to imitate the existing tests but I suppose
> > this may be useful in the future if that series gets merged.
>
> The fact that there is a series touching the same area might also hint
> that it might not be the right time to work on this.

I understand.

> > diff --git a/t/unit-tests/t-oid-array.c b/t/unit-tests/t-oid-array.c
> > new file mode 100644
> > index 0000000000..b4f43c025d
> > --- /dev/null
> > +++ b/t/unit-tests/t-oid-array.c
> > @@ -0,0 +1,222 @@
> > +#include "test-lib.h"
> > +#include "hex.h"
> > +#include "oid-array.h"
> > +#include "strbuf.h"
> > +
> > +#define INPUT "88", "44", "aa", "55"
> > +#define INPUT_DUP \
> > +       "88", "44", "aa", "55", "88", "44", "aa", "55", "88", "44", "aa", "55"
>
> Can you reuse INPUT in INPUT_DUP?

Yeah, that would be more clearer.

> > +#define INPUT_ONLY_DUP "55", "55"
> > +#define ENUMERATION_RESULT_SORTED "44", "55", "88", "aa"
> > +
> > +/*
> > + * allocates the memory based on the hash algorithm used and sets the length to
> > + * it.
> > + */
> > +static void hex_strbuf_init(struct strbuf *hex)
> > +{
> > +       static int sz = -1;
> > +
> > +       if (sz == -1) {
> > +               char *algo_env = getenv("GIT_TEST_DEFAULT_HASH");
> > +               if (algo_env && !strcmp(algo_env, "sha256"))
> > +                       sz = GIT_SHA256_HEXSZ;
> > +               else
> > +                       sz = GIT_SHA1_HEXSZ;
> > +       }
> > +
> > +       strbuf_init(hex, sz);
> > +       strbuf_setlen(hex, sz);
> > +}
>
> A strbuf can grow when we add stuff to it. We don't need to know its
> size in advance. So I am not sure this function is actually useful.

Yeah, this was mainly for deciding the hash algorithm but that logic can
be moved to fill_hex_strbuf.

> > +/* fills the hex strbuf with alternating characters from 'c' */
> > +static void fill_hex_strbuf(struct strbuf *hex, char *c)
> > +{
> > +       size_t i;
> > +       for (i = 0; i < hex->len; i++)
> > +               hex->buf[i] = (i & 1) ? c[1] : c[0];
>
> There is strbuf_addch() to add a single char to a strbuf, or
> strbuf_add() and strbuf_addstr() to add many chars at once.
Will update it.

Thank you for the feedback and review.
Christian Couder Feb. 27, 2024, 9:59 a.m. UTC | #4
On Mon, Feb 26, 2024 at 8:11 PM Ghanshyam Thakkar
<shyamthakkar001@gmail.com> wrote:
>
> On Mon Feb 26, 2024 at 8:41 PM IST, Christian Couder wrote:

> > So I think it would be better to work on other things instead, like
> > perhaps reviewing other people's work or working on other bug fixes or
> > features. Anyway now that this is on the mailing list, I might as well
> > review it as it could help with your application. But please consider
> > working on other things.
>
> I understand and will work on other things.

Thanks!

> > > In unit testing however, we do not
> > > need to initialize the repo. We can set the length of the hexadecimal
> > > strbuf according to the algorithm used directly.
> >
> > So is your patch doing that or not? It might be better to be explicit.
> > Also if 'strbuf's are used, then is it really worth it to set their
> > length in advance, instead of just letting them grow to the right
> > length as we add hex to them?
>
> I thought of it like this: If we were to just let them grow, then we
> would need separate logic for reusing that strbuf or use a different
> one everytime since it always grows. By separating allocation
> (hex_strbuf_init) and manipulation (fill_hex_strbuf), that same strbuf
> can be reused for different hex values.
>
> But, none of the test currently need to reuse the same strbuf, so I
> suppose it is better to just let it grow and even if the need arises we
> can use strbuf_splice().

It's not a problem to use a new strbuf for each different hex value.
Tests don't need a lot of performance as they are used mostly by
developers, not by everyone using Git. Also if you want to reuse a
strbuf, you can just use strbuf_reset() on it and then reuse it.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 78e874099d..5060d7eff3 100644
--- a/Makefile
+++ b/Makefile
@@ -820,7 +820,6 @@  TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
 TEST_BUILTINS_OBJS += test-match-trees.o
 TEST_BUILTINS_OBJS += test-mergesort.o
 TEST_BUILTINS_OBJS += test-mktemp.o
-TEST_BUILTINS_OBJS += test-oid-array.o
 TEST_BUILTINS_OBJS += test-oidmap.o
 TEST_BUILTINS_OBJS += test-oidtree.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
@@ -1346,6 +1345,7 @@  UNIT_TEST_PROGRAMS += t-mem-pool
 UNIT_TEST_PROGRAMS += t-strbuf
 UNIT_TEST_PROGRAMS += t-ctype
 UNIT_TEST_PROGRAMS += t-prio-queue
+UNIT_TEST_PROGRAMS += t-oid-array
 UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
 UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
 UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
diff --git a/t/helper/test-oid-array.c b/t/helper/test-oid-array.c
deleted file mode 100644
index aafe398ef0..0000000000
--- a/t/helper/test-oid-array.c
+++ /dev/null
@@ -1,45 +0,0 @@ 
-#include "test-tool.h"
-#include "hex.h"
-#include "oid-array.h"
-#include "setup.h"
-#include "strbuf.h"
-
-static int print_oid(const struct object_id *oid, void *data UNUSED)
-{
-	puts(oid_to_hex(oid));
-	return 0;
-}
-
-int cmd__oid_array(int argc UNUSED, const char **argv UNUSED)
-{
-	struct oid_array array = OID_ARRAY_INIT;
-	struct strbuf line = STRBUF_INIT;
-	int nongit_ok;
-
-	setup_git_directory_gently(&nongit_ok);
-
-	while (strbuf_getline(&line, stdin) != EOF) {
-		const char *arg;
-		struct object_id oid;
-
-		if (skip_prefix(line.buf, "append ", &arg)) {
-			if (get_oid_hex(arg, &oid))
-				die("not a hexadecimal oid: %s", arg);
-			oid_array_append(&array, &oid);
-		} else if (skip_prefix(line.buf, "lookup ", &arg)) {
-			if (get_oid_hex(arg, &oid))
-				die("not a hexadecimal oid: %s", arg);
-			printf("%d\n", oid_array_lookup(&array, &oid));
-		} else if (!strcmp(line.buf, "clear"))
-			oid_array_clear(&array);
-		else if (!strcmp(line.buf, "for_each_unique"))
-			oid_array_for_each_unique(&array, print_oid, NULL);
-		else
-			die("unknown command: %s", line.buf);
-	}
-
-	strbuf_release(&line);
-	oid_array_clear(&array);
-
-	return 0;
-}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 482a1e58a4..ad74bfffbe 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -42,7 +42,6 @@  static struct test_cmd cmds[] = {
 	{ "match-trees", cmd__match_trees },
 	{ "mergesort", cmd__mergesort },
 	{ "mktemp", cmd__mktemp },
-	{ "oid-array", cmd__oid_array },
 	{ "oidmap", cmd__oidmap },
 	{ "oidtree", cmd__oidtree },
 	{ "online-cpus", cmd__online_cpus },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index b1be7cfcf5..4f961a38c0 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -65,7 +65,6 @@  int cmd__scrap_cache_tree(int argc, const char **argv);
 int cmd__serve_v2(int argc, const char **argv);
 int cmd__sha1(int argc, const char **argv);
 int cmd__sha1_is_sha1dc(int argc, const char **argv);
-int cmd__oid_array(int argc, const char **argv);
 int cmd__sha256(int argc, const char **argv);
 int cmd__sigchain(int argc, const char **argv);
 int cmd__simple_ipc(int argc, const char **argv);
diff --git a/t/t0064-oid-array.sh b/t/t0064-oid-array.sh
deleted file mode 100755
index 88c89e8f48..0000000000
--- a/t/t0064-oid-array.sh
+++ /dev/null
@@ -1,104 +0,0 @@ 
-#!/bin/sh
-
-test_description='basic tests for the oid array implementation'
-
-TEST_PASSES_SANITIZE_LEAK=true
-. ./test-lib.sh
-
-echoid () {
-	prefix="${1:+$1 }"
-	shift
-	while test $# -gt 0
-	do
-		echo "$prefix$ZERO_OID" | sed -e "s/00/$1/g"
-		shift
-	done
-}
-
-test_expect_success 'ordered enumeration' '
-	echoid "" 44 55 88 aa >expect &&
-	{
-		echoid append 88 44 aa 55 &&
-		echo for_each_unique
-	} | test-tool oid-array >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'ordered enumeration with duplicate suppression' '
-	echoid "" 44 55 88 aa >expect &&
-	{
-		echoid append 88 44 aa 55 &&
-		echoid append 88 44 aa 55 &&
-		echoid append 88 44 aa 55 &&
-		echo for_each_unique
-	} | test-tool oid-array >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success 'lookup' '
-	{
-		echoid append 88 44 aa 55 &&
-		echoid lookup 55
-	} | test-tool oid-array >actual &&
-	n=$(cat actual) &&
-	test "$n" -eq 1
-'
-
-test_expect_success 'lookup non-existing entry' '
-	{
-		echoid append 88 44 aa 55 &&
-		echoid lookup 33
-	} | test-tool oid-array >actual &&
-	n=$(cat actual) &&
-	test "$n" -lt 0
-'
-
-test_expect_success 'lookup with duplicates' '
-	{
-		echoid append 88 44 aa 55 &&
-		echoid append 88 44 aa 55 &&
-		echoid append 88 44 aa 55 &&
-		echoid lookup 55
-	} | test-tool oid-array >actual &&
-	n=$(cat actual) &&
-	test "$n" -ge 3 &&
-	test "$n" -le 5
-'
-
-test_expect_success 'lookup non-existing entry with duplicates' '
-	{
-		echoid append 88 44 aa 55 &&
-		echoid append 88 44 aa 55 &&
-		echoid append 88 44 aa 55 &&
-		echoid lookup 66
-	} | test-tool oid-array >actual &&
-	n=$(cat actual) &&
-	test "$n" -lt 0
-'
-
-test_expect_success 'lookup with almost duplicate values' '
-	# n-1 5s
-	root=$(echoid "" 55) &&
-	root=${root%5} &&
-	{
-		id1="${root}5" &&
-		id2="${root}f" &&
-		echo "append $id1" &&
-		echo "append $id2" &&
-		echoid lookup 55
-	} | test-tool oid-array >actual &&
-	n=$(cat actual) &&
-	test "$n" -eq 0
-'
-
-test_expect_success 'lookup with single duplicate value' '
-	{
-		echoid append 55 55 &&
-		echoid lookup 55
-	} | test-tool oid-array >actual &&
-	n=$(cat actual) &&
-	test "$n" -ge 0 &&
-	test "$n" -le 1
-'
-
-test_done
diff --git a/t/unit-tests/t-oid-array.c b/t/unit-tests/t-oid-array.c
new file mode 100644
index 0000000000..b4f43c025d
--- /dev/null
+++ b/t/unit-tests/t-oid-array.c
@@ -0,0 +1,222 @@ 
+#include "test-lib.h"
+#include "hex.h"
+#include "oid-array.h"
+#include "strbuf.h"
+
+#define INPUT "88", "44", "aa", "55"
+#define INPUT_DUP \
+	"88", "44", "aa", "55", "88", "44", "aa", "55", "88", "44", "aa", "55"
+#define INPUT_ONLY_DUP "55", "55"
+#define ENUMERATION_RESULT_SORTED "44", "55", "88", "aa"
+
+/*
+ * allocates the memory based on the hash algorithm used and sets the length to
+ * it.
+ */
+static void hex_strbuf_init(struct strbuf *hex)
+{
+	static int sz = -1;
+
+	if (sz == -1) {
+		char *algo_env = getenv("GIT_TEST_DEFAULT_HASH");
+		if (algo_env && !strcmp(algo_env, "sha256"))
+			sz = GIT_SHA256_HEXSZ;
+		else
+			sz = GIT_SHA1_HEXSZ;
+	}
+
+	strbuf_init(hex, sz);
+	strbuf_setlen(hex, sz);
+}
+
+/* callback function for for_each used for printing */
+static int print_cb(const struct object_id *oid, void *data)
+{
+	int *i = data;
+	test_msg("%d. %s", *i, oid_to_hex(oid));
+	*i += 1;
+	return 0;
+}
+
+/* prints the oid_array with a message title */
+static void print_oid_array(struct oid_array *array, char *msg)
+{
+	int i = 0;
+	test_msg("%s", msg);
+	oid_array_for_each(array, print_cb, &i);
+}
+
+/* fills the hex strbuf with alternating characters from 'c' */
+static void fill_hex_strbuf(struct strbuf *hex, char *c)
+{
+	size_t i;
+	for (i = 0; i < hex->len; i++)
+		hex->buf[i] = (i & 1) ? c[1] : c[0];
+}
+
+/* populates object_id with hexadecimal representation generated from 'c' */
+static int get_oid_hex_input(struct object_id *oid, char *c)
+{
+	int ret;
+	struct strbuf hex;
+
+	hex_strbuf_init(&hex);
+	fill_hex_strbuf(&hex, c);
+	ret = get_oid_hex_any(hex.buf, oid);
+	if (ret == GIT_HASH_UNKNOWN)
+		test_msg("not a valid hexadecimal oid: %s", hex.buf);
+	strbuf_release(&hex);
+	return ret;
+}
+
+/* populates the oid_array with input from entries array */
+static int populate_oid_array(struct oid_array *oidarray, char **entries,
+			      size_t len)
+{
+	size_t i;
+	struct object_id oid;
+
+	for (i = 0; i < len; i++) {
+		if (!check_int(get_oid_hex_input(&oid, entries[i]), !=,
+			       GIT_HASH_UNKNOWN))
+			return -1;
+		oid_array_append(oidarray, &oid);
+	}
+	return 0;
+}
+
+/* callback function for enumeration test */
+static int add_to_oid_array(const struct object_id *oid, void *data)
+{
+	struct oid_array *array = data;
+	oid_array_append(array, oid);
+	return 0;
+}
+
+static void test_enumeration(char *input_entries[], size_t input_size,
+			     char *expected_entries[], size_t expected_size)
+{
+	int i;
+	struct oid_array input = OID_ARRAY_INIT;
+	struct oid_array actual = OID_ARRAY_INIT;
+	struct oid_array expect = OID_ARRAY_INIT;
+
+	if (populate_oid_array(&input, input_entries, input_size) == -1)
+		goto cleanup;
+
+	if (populate_oid_array(&expect, expected_entries, expected_size) == -1)
+		goto cleanup;
+
+	oid_array_for_each_unique(&input, add_to_oid_array, &actual);
+	if (!check_int(expect.nr, ==, expected_size) ||
+	    !check_int(actual.nr, ==, expected_size))
+		goto cleanup;
+
+	for (i = 0; i < expected_size; i++) {
+		if (!check_int(oideq(&actual.oid[i], &expect.oid[i]), ==, 1)) {
+			test_msg("failed at index %d", i);
+			print_oid_array(&expect, "expected array content:");
+			print_oid_array(&actual, "actual array content:");
+			goto cleanup;
+		}
+	}
+
+cleanup:
+	oid_array_clear(&input);
+	oid_array_clear(&actual);
+	oid_array_clear(&expect);
+}
+
+#define ENUMERATION_INPUT(INPUT, RESULT, NAME)                                \
+	static void t_ordered_enumeration_##NAME(void)                        \
+	{                                                                     \
+		char *input_entries[] = { INPUT };                            \
+		char *expected_entries[] = { RESULT };                        \
+		size_t input_size = ARRAY_SIZE(input_entries);                \
+		size_t expected_size = ARRAY_SIZE(expected_entries);          \
+		test_enumeration(input_entries, input_size, expected_entries, \
+				 expected_size);                              \
+	}
+
+ENUMERATION_INPUT(INPUT, ENUMERATION_RESULT_SORTED, non_duplicate)
+ENUMERATION_INPUT(INPUT_DUP, ENUMERATION_RESULT_SORTED, duplicate)
+
+static void lookup_setup(void (*f)(struct strbuf *buf, struct oid_array *array))
+{
+	struct oid_array array = OID_ARRAY_INIT;
+	struct strbuf buf;
+
+	hex_strbuf_init(&buf);
+	f(&buf, &array);
+	oid_array_clear(&array);
+	strbuf_release(&buf);
+}
+
+#define LOOKUP_INPUT(INPUT, QUERY, NAME, CONDITION)                           \
+	static void t_##NAME(struct strbuf *buf UNUSED,                       \
+			     struct oid_array *array)                         \
+	{                                                                     \
+		struct object_id oid_query;                                   \
+		char *input_entries[] = { INPUT };                            \
+		size_t input_size = ARRAY_SIZE(input_entries);                \
+		int ret;                                                      \
+		if (!check_int(get_oid_hex_input(&oid_query, QUERY), !=,      \
+			       GIT_HASH_UNKNOWN))                             \
+			return;                                               \
+		if (!check_int(populate_oid_array(array, input_entries,       \
+						  input_size),                \
+			       !=, -1))                                       \
+			return;                                               \
+		ret = oid_array_lookup(array, &oid_query);                    \
+		if (!check(CONDITION)) {                                      \
+			print_oid_array(array, "array content:");             \
+			test_msg("oid query for lookup: %s", oid_query.hash); \
+		}                                                             \
+	}
+
+/* ret is return value of oid_array_lookup() */
+LOOKUP_INPUT(INPUT, "55", lookup, ret == 1)
+LOOKUP_INPUT(INPUT, "33", lookup_nonexist, ret < 1)
+LOOKUP_INPUT(INPUT_DUP, "66", lookup_nonexist_dup, ret < 0)
+LOOKUP_INPUT(INPUT_DUP, "55", lookup_dup, ret >= 3 && ret <= 5)
+LOOKUP_INPUT(INPUT_ONLY_DUP, "55", lookup_only_dup, ret >= 0 && ret <= 1)
+
+static void t_lookup_almost_dup(struct strbuf *hex, struct oid_array *array)
+{
+	struct object_id oid;
+
+	fill_hex_strbuf(hex, "55");
+	if (!check_int(get_oid_hex_any(hex->buf, &oid), !=, GIT_HASH_UNKNOWN))
+		return;
+
+	oid_array_append(array, &oid);
+	/* last character different */
+	hex->buf[hex->len - 1] = 'f';
+	if (!check_int(get_oid_hex_any(hex->buf, &oid), !=, GIT_HASH_UNKNOWN))
+		return;
+
+	oid_array_append(array, &oid);
+	if (!check_int(oid_array_lookup(array, &oid), ==, 1)) {
+		print_oid_array(array, "array content:");
+		test_msg("oid query for lookup: %s", hex->buf);
+	}
+}
+
+int cmd_main(int argc, const char **argv)
+{
+	TEST(t_ordered_enumeration_non_duplicate(),
+	     "ordered enumeration works");
+	TEST(t_ordered_enumeration_duplicate(),
+	     "ordered enumeration with duplicate suppresion works");
+	TEST(lookup_setup(t_lookup), "lookup works");
+	TEST(lookup_setup(t_lookup_nonexist), "lookup non-existant entry");
+	TEST(lookup_setup(t_lookup_dup), "lookup with duplicates works");
+	TEST(lookup_setup(t_lookup_nonexist_dup),
+	     "lookup non-existant entry with duplicates");
+	TEST(lookup_setup(t_lookup_almost_dup),
+	     "lookup with almost duplicate values works");
+	TEST(lookup_setup(t_lookup_only_dup),
+	     "lookup with single duplicate value works");
+
+	return test_done();
+}