mbox series

[GSoC,v2,0/7] t: port reftable/merged_test.c to the unit testing framework

Message ID 20240709053847.4453-1-chandrapratap3519@gmail.com (mailing list archive)
Headers show
Series t: port reftable/merged_test.c to the unit testing framework | expand

Message

Chandra Pratap July 9, 2024, 5:28 a.m. UTC
In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
testing framework written entirely in C was introduced to the Git project
aimed at simplifying testing and reducing test run times.
Currently, tests for the reftable refs-backend are performed by a custom
testing framework defined by reftable/test_framework.{c, h}. Port
reftable/merged_test.c to the unit testing framework and improve upon
the ported test.

The first patch in the series moves the test to the unit testing framework,
and the rest of the patches improve upon the ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

---
Changes in v2:
- Rename test functions & description to be more explanatory
- Remove a redundant comment in patch 1
- Add the 3rd patch which makes improvements to a test
- Add the 4th patch which makes apt function parameters 'const'
- Remove a redundant check in patch 6

CI/PR: https://github.com/gitgitgadget/git/pull/1755

Chandra Pratap (7):
[PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework
[PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines
[PATCH 3/7] t-reftable-merged: improve the test for t_merged_single_record()
[PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions
[PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index
[PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records
[PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR

Makefile                                                   |   2 +-
t/helper/test-reftable.c                                   |   1 -
reftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 202 +++++++++++++++----------------
3 files changed, 103 insertions(+), 102 deletions(-)

Range-diff against v2:
1:  f4d5da52f5 ! 1:  0d71deffad t: move reftable/merged_test.c to the unit testing framework
    @@ t/unit-tests/t-reftable-merged.c: static void readers_destroy(struct reftable_re
      }
      
     -static void test_merged_between(void)
    -+static void t_merged_between(void)
    ++static void t_merged_single_record(void)
      {
      	struct reftable_ref_record r1[] = { {
      		.refname = (char *) "b",
    @@ t/unit-tests/t-reftable-merged.c: static void test_merged_between(void)
      }
      
     -static void test_merged(void)
    -+static void t_merged(void)
    ++static void t_merged_refs(void)
      {
      	struct reftable_ref_record r1[] = {
      		{
    @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)
      
      	reftable_reader_free(rd);
      	reftable_merged_table_free(merged);
    -@@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)
    + 	strbuf_release(&buf);
    + }
      
    - /* XXX test refs_for(oid) */
    +-/* XXX test refs_for(oid) */
      
     -int merged_test_main(int argc, const char *argv[])
     +int cmd_main(int argc, const char *argv[])
    @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)
     -	RUN_TEST(test_merged);
     -	RUN_TEST(test_default_write_opts);
     -	return 0;
    -+	TEST(t_merged_logs(), "merged table with log records");
    -+	TEST(t_merged_between(), "seek ref in a merged table");
    -+	TEST(t_merged(), "merged table with multiple updates to same ref");
     +	TEST(t_default_write_opts(), "merged table with default write opts");
    ++	TEST(t_merged_logs(), "merged table with multiple log updates for same ref");
    ++	TEST(t_merged_refs(), "merged table with multiple updates to same ref");
    ++	TEST(t_merged_single_record(), "ref ocurring in only one record can be fetched");
     +
     +	return test_done();
      }
2:  fb0f0946b4 ! 2:  a449e2edcf t: harmonize t-reftable-merged.c with coding guidelines
    @@ t/unit-tests/t-reftable-merged.c: merged_table_from_records(struct reftable_ref_
      		reftable_reader_free(readers[i]);
      	reftable_free(readers);
      }
    -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
    +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)
      	struct reftable_reader **readers = NULL;
      	struct reftable_merged_table *mt =
      		merged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);
    @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
      	int err;
      
      	merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
    -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
    +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)
      	reftable_iterator_destroy(&it);
      	readers_destroy(readers, 2);
      	reftable_merged_table_free(mt);
    @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
      	reftable_free(bs);
      }
      
    -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
    +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
      	struct reftable_reader **readers = NULL;
      	struct reftable_merged_table *mt =
      		merged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);
    @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
      
      	merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
      	err = reftable_iterator_seek_ref(&it, "a");
    -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
    +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
      	check_int(reftable_merged_table_min_update_index(mt), ==, 1);
      
      	while (len < 100) { /* cap loops/recursion. */
    @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
      		int err = reftable_iterator_next_ref(&it, &ref);
      		if (err > 0)
      			break;
    -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
    +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
      	reftable_iterator_destroy(&it);
      
      	check_int(ARRAY_SIZE(want), ==, len);
-:  ---------- > 3:  bba993bb26 t-reftable-merged: improve the test t_merged_single_record()
-:  ---------- > 4:  4d508eaa02 t-reftable-merged: improve the const-correctness of helper functions
3:  85731b6358 ! 5:  1c9ba26c22 t-reftable-merged: add tests for reftable_merged_table_max_update_index
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
     
      ## t/unit-tests/t-reftable-merged.c ##
    -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
    +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
      	check(!err);
      	check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
      	check_int(reftable_merged_table_min_update_index(mt), ==, 1);
4:  5d6fd842ac < -:  ---------- t-reftable-merged: use reftable_ref_record_equal to compare ref records
-:  ---------- > 6:  309c7f412e t-reftable-merged: use reftable_ref_record_equal to compare ref records
5:  d8cbb73533 = 7:  2cd7f5b0b4 t-reftable-merged: add test for REFTABLE_FORMAT_ERROR

Comments

Karthik Nayak July 10, 2024, 9:19 a.m. UTC | #1
Chandra Pratap <chandrapratap3519@gmail.com> writes:

> In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
> testing framework written entirely in C was introduced to the Git project
> aimed at simplifying testing and reducing test run times.
> Currently, tests for the reftable refs-backend are performed by a custom
> testing framework defined by reftable/test_framework.{c, h}. Port
> reftable/merged_test.c to the unit testing framework and improve upon
> the ported test.
>
> The first patch in the series moves the test to the unit testing framework,
> and the rest of the patches improve upon the ported test.
>

This series looks good now, sans a typo and Justin's comment!
Thanks!

> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
> ---
> Changes in v2:
> - Rename test functions & description to be more explanatory
> - Remove a redundant comment in patch 1
> - Add the 3rd patch which makes improvements to a test
> - Add the 4th patch which makes apt function parameters 'const'
> - Remove a redundant check in patch 6
>
> CI/PR: https://github.com/gitgitgadget/git/pull/1755
>
> Chandra Pratap (7):
> [PATCH 1/7] t: move reftable/merged_test.c to the unit testing framework
> [PATCH 2/7] t: harmonize t-reftable-merged.c with coding guidelines
> [PATCH 3/7] t-reftable-merged: improve the test for t_merged_single_record()
> [PATCH 4/7] t-reftable-merged: improve the const-correctness of helper functions
> [PATCH 5/7] t-reftable-merged: add tests for reftable_merged_table_max_update_index
> [PATCH 6/7] t-reftable-merged: use reftable_ref_record_equal to compare ref records
> [PATCH 7/7] t-reftable-merged: add test for REFTABLE_FORMAT_ERROR
>
> Makefile                                                   |   2 +-
> t/helper/test-reftable.c                                   |   1 -
> reftable/merged_test.c => t/unit-tests/t-reftable-merged.c | 202 +++++++++++++++----------------
> 3 files changed, 103 insertions(+), 102 deletions(-)
>
> Range-diff against v2:
> 1:  f4d5da52f5 ! 1:  0d71deffad t: move reftable/merged_test.c to the unit testing framework
>     @@ t/unit-tests/t-reftable-merged.c: static void readers_destroy(struct reftable_re
>       }
>
>      -static void test_merged_between(void)
>     -+static void t_merged_between(void)
>     ++static void t_merged_single_record(void)
>       {
>       	struct reftable_ref_record r1[] = { {
>       		.refname = (char *) "b",
>     @@ t/unit-tests/t-reftable-merged.c: static void test_merged_between(void)
>       }
>
>      -static void test_merged(void)
>     -+static void t_merged(void)
>     ++static void t_merged_refs(void)
>       {
>       	struct reftable_ref_record r1[] = {
>       		{
>     @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)
>
>       	reftable_reader_free(rd);
>       	reftable_merged_table_free(merged);
>     -@@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)
>     + 	strbuf_release(&buf);
>     + }
>
>     - /* XXX test refs_for(oid) */
>     +-/* XXX test refs_for(oid) */
>
>      -int merged_test_main(int argc, const char *argv[])
>      +int cmd_main(int argc, const char *argv[])
>     @@ t/unit-tests/t-reftable-merged.c: static void test_default_write_opts(void)
>      -	RUN_TEST(test_merged);
>      -	RUN_TEST(test_default_write_opts);
>      -	return 0;
>     -+	TEST(t_merged_logs(), "merged table with log records");
>     -+	TEST(t_merged_between(), "seek ref in a merged table");
>     -+	TEST(t_merged(), "merged table with multiple updates to same ref");
>      +	TEST(t_default_write_opts(), "merged table with default write opts");
>     ++	TEST(t_merged_logs(), "merged table with multiple log updates for same ref");
>     ++	TEST(t_merged_refs(), "merged table with multiple updates to same ref");
>     ++	TEST(t_merged_single_record(), "ref ocurring in only one record can be fetched");
>      +
>      +	return test_done();
>       }
> 2:  fb0f0946b4 ! 2:  a449e2edcf t: harmonize t-reftable-merged.c with coding guidelines
>     @@ t/unit-tests/t-reftable-merged.c: merged_table_from_records(struct reftable_ref_
>       		reftable_reader_free(readers[i]);
>       	reftable_free(readers);
>       }
>     -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
>     +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)
>       	struct reftable_reader **readers = NULL;
>       	struct reftable_merged_table *mt =
>       		merged_table_from_records(refs, &bs, &readers, sizes, bufs, 2);
>     @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
>       	int err;
>
>       	merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
>     -@@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
>     +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_single_record(void)
>       	reftable_iterator_destroy(&it);
>       	readers_destroy(readers, 2);
>       	reftable_merged_table_free(mt);
>     @@ t/unit-tests/t-reftable-merged.c: static void t_merged_between(void)
>       	reftable_free(bs);
>       }
>
>     -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
>     +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
>       	struct reftable_reader **readers = NULL;
>       	struct reftable_merged_table *mt =
>       		merged_table_from_records(refs, &bs, &readers, sizes, bufs, 3);
>     @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
>
>       	merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
>       	err = reftable_iterator_seek_ref(&it, "a");
>     -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
>     +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
>       	check_int(reftable_merged_table_min_update_index(mt), ==, 1);
>
>       	while (len < 100) { /* cap loops/recursion. */
>     @@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
>       		int err = reftable_iterator_next_ref(&it, &ref);
>       		if (err > 0)
>       			break;
>     -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
>     +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
>       	reftable_iterator_destroy(&it);
>
>       	check_int(ARRAY_SIZE(want), ==, len);
> -:  ---------- > 3:  bba993bb26 t-reftable-merged: improve the test t_merged_single_record()
> -:  ---------- > 4:  4d508eaa02 t-reftable-merged: improve the const-correctness of helper functions
> 3:  85731b6358 ! 5:  1c9ba26c22 t-reftable-merged: add tests for reftable_merged_table_max_update_index
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-merged.c ##
>     -@@ t/unit-tests/t-reftable-merged.c: static void t_merged(void)
>     +@@ t/unit-tests/t-reftable-merged.c: static void t_merged_refs(void)
>       	check(!err);
>       	check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
>       	check_int(reftable_merged_table_min_update_index(mt), ==, 1);
> 4:  5d6fd842ac < -:  ---------- t-reftable-merged: use reftable_ref_record_equal to compare ref records
> -:  ---------- > 6:  309c7f412e t-reftable-merged: use reftable_ref_record_equal to compare ref records
> 5:  d8cbb73533 = 7:  2cd7f5b0b4 t-reftable-merged: add test for REFTABLE_FORMAT_ERROR
Junio C Hamano July 11, 2024, 2:38 p.m. UTC | #2
Karthik Nayak <karthik.188@gmail.com> writes:

> This series looks good now, sans a typo and Justin's comment!
> Thanks!

Thanks, all, for helping the series (and Chandra) along.