diff mbox series

[09/11] t-reftable-record: add index tests for reftable_record_is_deletion()

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

Commit Message

Chandra Pratap June 21, 2024, 11:51 a.m. UTC
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 index records.
Note that since index records cannot be of type deletion, this function
must always return '0' when called on an index record.

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 | 1 +
 1 file changed, 1 insertion(+)

Comments

Karthik Nayak June 25, 2024, 9:19 a.m. UTC | #1
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

Nit: 'In the current testing setup' holds true for the series, but on a
commit level, this statement needs to be modified with each tackled
type.

> is left untested for all the four record types (ref, log, obj, index).
>
> Add tests for this function in the case of index records.
> Note that since index records cannot be of type deletion, this function
> must always return '0' when called on an index record.
>
> 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 | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
> index 766431ca67..99ebfafe0b 100644
> --- a/t/unit-tests/t-reftable-record.c
> +++ b/t/unit-tests/t-reftable-record.c
> @@ -477,6 +477,7 @@ static void test_reftable_index_record_roundtrip(void)
>  	reftable_record_key(&in, &key);
>  	test_copy(&in);
>
> +	check(!reftable_record_is_deletion(&in));
>  	check(!strbuf_cmp(&key, &in.u.idx.last_key));
>  	n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ);
>  	check_int(n, >, 0);
> --
> 2.45.2.404.g9eaef5822c
Chandra Pratap June 25, 2024, 2:11 p.m. UTC | #2
On Tue, 25 Jun 2024 at 14:49, Karthik Nayak <karthik.188@gmail.com> wrote:
>
> 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
>
> Nit: 'In the current testing setup' holds true for the series, but on a
> commit level, this statement needs to be modified with each tackled
> type.

I'm not quite sure I follow. Could you explain this a bit further?
Karthik Nayak June 26, 2024, 11:59 a.m. UTC | #3
Chandra Pratap <chandrapratap3519@gmail.com> writes:

> On Tue, 25 Jun 2024 at 14:49, Karthik Nayak <karthik.188@gmail.com> wrote:
>>
>> 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
>>
>> Nit: 'In the current testing setup' holds true for the series, but on a
>> commit level, this statement needs to be modified with each tackled
>> type.
>
> I'm not quite sure I follow. Could you explain this a bit further?

Patch 06/11 says "In the current testing setup, this function is left
untested for all the four record types (ref, log, obj, index)." then it
proceeds to add tests for the 'index' type.

However 07/11 still says "In the current testing setup, this function is
left untested for all the four record types (ref, log, obj, index).",
which is no longer the state. Since we tackled 'index' in the previous
commit. So forth for the next two commits.
diff mbox series

Patch

diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
index 766431ca67..99ebfafe0b 100644
--- a/t/unit-tests/t-reftable-record.c
+++ b/t/unit-tests/t-reftable-record.c
@@ -477,6 +477,7 @@  static void test_reftable_index_record_roundtrip(void)
 	reftable_record_key(&in, &key);
 	test_copy(&in);
 
+	check(!reftable_record_is_deletion(&in));
 	check(!strbuf_cmp(&key, &in.u.idx.last_key));
 	n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ);
 	check_int(n, >, 0);