diff mbox series

[v3,10/11] t-reftable-record: add tests for reftable_ref_record_compare_name()

Message ID 20240628063625.4092-11-chandrapratap3519@gmail.com (mailing list archive)
State New
Headers show
Series t: port reftable/record_test.c to the unit testing framework | expand

Commit Message

Chandra Pratap June 28, 2024, 6:19 a.m. UTC
reftable_ref_record_compare_name() is a function defined by
reftable/record.{c, h} and is used to compare the refname of two
ref records when sorting multiple ref records using 'qsort'.
In the current testing setup, this function is left unexercised.
Add a testing function for the same.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 t/unit-tests/t-reftable-record.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Karthik Nayak June 30, 2024, 6:59 p.m. UTC | #1
Chandra Pratap <chandrapratap3519@gmail.com> writes:

> reftable_ref_record_compare_name() is a function defined by
> reftable/record.{c, h} and is used to compare the refname of two
> ref records when sorting multiple ref records using 'qsort'.
> In the current testing setup, this function is left unexercised.
> Add a testing function for the same.
>
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
>  t/unit-tests/t-reftable-record.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
> index 55b8d03494..f45f2fdef2 100644
> --- a/t/unit-tests/t-reftable-record.c
> +++ b/t/unit-tests/t-reftable-record.c
> @@ -95,6 +95,28 @@ static void test_reftable_ref_record_comparison(void)
>  	check(!reftable_record_cmp(&in[0], &in[1]));
>  }
>
> +static void test_reftable_ref_record_compare_name(void)
> +{
> +	struct reftable_ref_record recs[14] = { 0 };
> +	size_t N = ARRAY_SIZE(recs), i;
> +
> +	for (i = 0; i < N; i++)
> +		recs[i].refname = xstrfmt("%02"PRIuMAX, (uintmax_t)i);

This needs to be free'd too right?

So we create an array of 14 records, with refnames "00", "01", "02" ...
"13", here.

> +
> +	QSORT(recs, N, reftable_ref_record_compare_name);
> +

We then use `reftable_ref_record_compare_name` as the comparison
function to sort them.

> +	for (i = 1; i < N; i++) {
> +		check_int(strcmp(recs[i - 1].refname, recs[i].refname), <, 0);
> +		check_int(reftable_ref_record_compare_name(&recs[i], &recs[i]), ==, 0);
> +	}

Here we use `strcmp` to ensure that the ordering done by
`reftable_ref_record_compare_name` is correct. This makes sense,
although I would have expected this to be done the other way around.
i.e. we should use `strcmp` as the function used in `QSORT` and in this
loop we validate that `reftable_ref_record_compare_name` also produces
the same result when comparing.

> +
> +	for (i = 0; i < N - 1; i++)
> +		check_int(reftable_ref_record_compare_name(&recs[i + 1], &recs[i]), >, 0);
> +

Also, with the current setup, we use `reftable_ref_record_compare_name`
to sort the first array and then use `reftable_ref_record_compare_name`
to check if it is correct? This doesn't work, we need to isolate the
data creation from the inference, if the same function can influence
both, then we are not really testing the function.

> +	for (i = 0; i < N; i++)
> +		reftable_ref_record_release(&recs[i]);
> +}
> +

Nit: The top three loops could possibly be combined.

>  static void test_reftable_ref_record_roundtrip(void)
>  {
>  	struct strbuf scratch = STRBUF_INIT;
> @@ -490,6 +512,7 @@ int cmd_main(int argc, const char *argv[])
>  	TEST(test_reftable_log_record_comparison(), "comparison operations work on log record");
>  	TEST(test_reftable_index_record_comparison(), "comparison operations work on index record");
>  	TEST(test_reftable_obj_record_comparison(), "comparison operations work on obj record");
> +	TEST(test_reftable_ref_record_compare_name(), "reftable_ref_record_compare_name works");
>  	TEST(test_reftable_log_record_roundtrip(), "record operations work on log record");
>  	TEST(test_reftable_ref_record_roundtrip(), "record operations work on ref record");
>  	TEST(test_varint_roundtrip(), "put_var_int and get_var_int work");
> --
> 2.45.2.404.g9eaef5822c
diff mbox series

Patch

diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
index 55b8d03494..f45f2fdef2 100644
--- a/t/unit-tests/t-reftable-record.c
+++ b/t/unit-tests/t-reftable-record.c
@@ -95,6 +95,28 @@  static void test_reftable_ref_record_comparison(void)
 	check(!reftable_record_cmp(&in[0], &in[1]));
 }
 
+static void test_reftable_ref_record_compare_name(void)
+{
+	struct reftable_ref_record recs[14] = { 0 };
+	size_t N = ARRAY_SIZE(recs), i;
+
+	for (i = 0; i < N; i++)
+		recs[i].refname = xstrfmt("%02"PRIuMAX, (uintmax_t)i);
+
+	QSORT(recs, N, reftable_ref_record_compare_name);
+
+	for (i = 1; i < N; i++) {
+		check_int(strcmp(recs[i - 1].refname, recs[i].refname), <, 0);
+		check_int(reftable_ref_record_compare_name(&recs[i], &recs[i]), ==, 0);
+	}
+
+	for (i = 0; i < N - 1; i++)
+		check_int(reftable_ref_record_compare_name(&recs[i + 1], &recs[i]), >, 0);
+
+	for (i = 0; i < N; i++)
+		reftable_ref_record_release(&recs[i]);
+}
+
 static void test_reftable_ref_record_roundtrip(void)
 {
 	struct strbuf scratch = STRBUF_INIT;
@@ -490,6 +512,7 @@  int cmd_main(int argc, const char *argv[])
 	TEST(test_reftable_log_record_comparison(), "comparison operations work on log record");
 	TEST(test_reftable_index_record_comparison(), "comparison operations work on index record");
 	TEST(test_reftable_obj_record_comparison(), "comparison operations work on obj record");
+	TEST(test_reftable_ref_record_compare_name(), "reftable_ref_record_compare_name works");
 	TEST(test_reftable_log_record_roundtrip(), "record operations work on log record");
 	TEST(test_reftable_ref_record_roundtrip(), "record operations work on ref record");
 	TEST(test_varint_roundtrip(), "put_var_int and get_var_int work");