Re: [PATCH 06/11] t-reftable-record: add ref tests for reftable_record_is_deletion()

[Karthik Nayak <karthik.188@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

Chandra Pratap <chandrapratap3519@gmail.com> writes:

> reftable_record_is_deletion() is a function defined in
> reftable/record.{c, h} that determines whether a record is of
> type deletion or not. In the current testing setup, this function
> is left untested for all the four record types (ref, log, obj, index).
>
> Add tests for this function in the case of ref records.
>
> 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 | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
> index 1f9c830631..cbc2ce93b2 100644
> --- a/t/unit-tests/t-reftable-record.c
> +++ b/t/unit-tests/t-reftable-record.c
> @@ -108,6 +108,7 @@ static void test_reftable_ref_record_roundtrip(void)
>  	for (int i = REFTABLE_REF_DELETION; i < REFTABLE_NR_REF_VALUETYPES; i++) {
>  		struct reftable_record in = {
>  			.type = BLOCK_TYPE_REF,
> +			.u.ref.value_type = i,
>  		};
>  		struct reftable_record out = { .type = BLOCK_TYPE_REF };
>  		struct strbuf key = STRBUF_INIT;
> @@ -121,15 +122,19 @@ static void test_reftable_ref_record_roundtrip(void)
>  		in.u.ref.value_type = i;
>  		switch (i) {
>  		case REFTABLE_REF_DELETION:
> +			check(reftable_record_is_deletion(&in));
>  			break;
>  		case REFTABLE_REF_VAL1:
> +			check(!reftable_record_is_deletion(&in));
>  			set_hash(in.u.ref.value.val1, 1);
>  			break;
>  		case REFTABLE_REF_VAL2:
> +			check(!reftable_record_is_deletion(&in));
>  			set_hash(in.u.ref.value.val2.value, 1);
>  			set_hash(in.u.ref.value.val2.target_value, 2);
>  			break;
>  		case REFTABLE_REF_SYMREF:
> +			check(!reftable_record_is_deletion(&in));
>  			in.u.ref.value.symref = xstrdup("target");
>  			break;
>  		}

I think it might be easier to follow if we just move this outside the
switch, perhaps something like:

diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
index d480cc438a..5cb910f6be 100644
--- a/t/unit-tests/t-reftable-record.c
+++ b/t/unit-tests/t-reftable-record.c
@@ -139,19 +139,15 @@ static void test_reftable_ref_record_roundtrip(void)
 		in.u.ref.value_type = i;
 		switch (i) {
 		case REFTABLE_REF_DELETION:
-			check(reftable_record_is_deletion(&in));
 			break;
 		case REFTABLE_REF_VAL1:
-			check(!reftable_record_is_deletion(&in));
 			set_hash(in.u.ref.value.val1, 1);
 			break;
 		case REFTABLE_REF_VAL2:
-			check(!reftable_record_is_deletion(&in));
 			set_hash(in.u.ref.value.val2.value, 1);
 			set_hash(in.u.ref.value.val2.target_value, 2);
 			break;
 		case REFTABLE_REF_SYMREF:
-			check(!reftable_record_is_deletion(&in));
 			in.u.ref.value.symref = xstrdup("target");
 			break;
 		}
@@ -159,6 +155,7 @@ static void test_reftable_ref_record_roundtrip(void)

 		test_copy(&in);

+		check_int(reftable_record_is_deletion(&in), ==, i == REFTABLE_REF_DELETION);
 		check_int(reftable_record_val_type(&in), ==, i);

 		reftable_record_key(&in, &key);


> --
> 2.45.2.404.g9eaef5822c

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]