mbox series

[GSoC,v4,0/11] t: port reftable/record_test.c to the unit testing framework framework

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

Message

Chandra Pratap July 2, 2024, 7:22 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/record_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 v4:
- Rename the tests to follow a 't_foo()' pattern instead
  of 'test_foo()'
- Use hard-coded test input in the 10th and 11th patch

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

Chandra Pratap (11):
t: move reftable/record_test.c to the unit testing framework
t-reftable-record: add reftable_record_cmp() tests for log records
t-reftable-record: add comparison tests for ref records
t-reftable-record: add comparison tests for index records
t-reftable-record: add comparison tests for obj records
t-reftable-record: add reftable_record_is_deletion() test for ref records
t-reftable-record: add reftable_record_is_deletion() test for log records
t-reftable-record: add reftable_record_is_deletion() test for obj records
t-reftable-record: add reftable_record_is_deletion() test for index records
t-reftable-record: add tests for reftable_ref_record_compare_name()
t-reftable-record: add tests for reftable_log_record_compare_key()

Makefile                         |   2 +-
reftable/record_test.c           | 382 -------------------------
t/helper/test-reftable.c         |   1 -
t/unit-tests/t-reftable-record.c | 551 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 552 insertions(+), 384 deletions(-)

Range-diff against v3:
 1:  c88fb5bcfa !  1:  c210a0da8f t: move reftable/record_test.c to the unit testing framework
    @@ Commit message
         reftable/record.{c, h}. Migrate reftable/record_test.c to the
         unit testing framework. Migration involves refactoring the tests
         to use the unit testing framework instead of reftable's test
    -    framework.
    +    framework, and renaming the tests to fit unit-tests' naming scheme.
    +
         While at it, change the type of index variable 'i' to 'size_t'
         from 'int'. This is because 'i' is used in comparison against
         'ARRAY_SIZE(x)' which is of type 'size_t'.
    @@ reftable/record_test.c => t/unit-tests/t-reftable-record.c
      */

     -#include "record.h"
    --
    ++#include "test-lib.h"
    ++#include "reftable/constants.h"
    ++#include "reftable/record.h"
    +
     -#include "system.h"
     -#include "basics.h"
     -#include "constants.h"
     -#include "test_framework.h"
     -#include "reftable-tests.h"
    -+#include "test-lib.h"
    -+#include "reftable/constants.h"
    -+#include "reftable/record.h"
    -
    - static void test_copy(struct reftable_record *rec)
    +-
    +-static void test_copy(struct reftable_record *rec)
    ++static void t_copy(struct reftable_record *rec)
      {
    + 	struct reftable_record copy;
    + 	uint8_t typ;
     @@ t/unit-tests/t-reftable-record.c: static void test_copy(struct reftable_record *rec)
      	reftable_record_copy_from(&copy, rec, GIT_SHA1_RAWSZ);
      	/* do it twice to catch memory leaks */
    @@ t/unit-tests/t-reftable-record.c: static void test_copy(struct reftable_record *

      	reftable_record_release(&copy);
      }
    +
    +-static void test_varint_roundtrip(void)
    ++static void t_varint_roundtrip(void)
    + {
    + 	uint64_t inputs[] = { 0,
    + 			      1,
     @@ t/unit-tests/t-reftable-record.c: static void test_varint_roundtrip(void)
      			      4096,
      			      ((uint64_t)1 << 63),
    @@ t/unit-tests/t-reftable-record.c: static void test_varint_roundtrip(void)
     -	}
      }

    - static void test_reftable_ref_record_roundtrip(void)
    +-static void test_reftable_ref_record_roundtrip(void)
    ++static void t_reftable_ref_record_roundtrip(void)
      {
      	struct strbuf scratch = STRBUF_INIT;
     -	int i = 0;
    @@ t/unit-tests/t-reftable-record.c: static void test_varint_roundtrip(void)
      			.type = BLOCK_TYPE_REF,
      		};
     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
    + 		}
    + 		in.u.ref.refname = xstrdup("refs/heads/master");

    - 		test_copy(&in);
    +-		test_copy(&in);
    ++		t_copy(&in);

     -		EXPECT(reftable_record_val_type(&in) == i);
     +		check_int(reftable_record_val_type(&in), ==, i);
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip
      						 GIT_SHA1_RAWSZ));
      		reftable_record_release(&in);

    +@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
    + 	strbuf_release(&scratch);
    + }
    +
    +-static void test_reftable_log_record_equal(void)
    ++static void t_reftable_log_record_equal(void)
    + {
    + 	struct reftable_log_record in[2] = {
    + 		{
     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_equal(void)
      		}
      	};
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_equal(voi
      	reftable_log_record_release(&in[1]);
      }

    - static void test_reftable_log_record_roundtrip(void)
    +-static void test_reftable_log_record_roundtrip(void)
    ++static void t_reftable_log_record_roundtrip(void)
      {
     -	int i;
      	struct reftable_log_record in[] = {
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip
      		struct strbuf key = STRBUF_INIT;
      		uint8_t buffer[1024] = { 0 };
     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip(void)
    +
    + 		rec.u.log = in[i];
    +
    +-		test_copy(&rec);
    ++		t_copy(&rec);
    +
      		reftable_record_key(&rec, &key);

      		n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ);
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip
      						 GIT_SHA1_RAWSZ));
      		reftable_log_record_release(&in[i]);
      		strbuf_release(&key);
    +@@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip(void)
    + 	strbuf_release(&scratch);
    + }
    +
    +-static void test_key_roundtrip(void)
    ++static void t_key_roundtrip(void)
    + {
    + 	uint8_t buffer[1024] = { 0 };
    + 	struct string_view dest = {
     @@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
      	strbuf_addstr(&key, "refs/tags/bla");
      	extra = 6;
    @@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)

      	strbuf_release(&last_key);
      	strbuf_release(&key);
    + 	strbuf_release(&roundtrip);
    + }
    +
    +-static void test_reftable_obj_record_roundtrip(void)
    ++static void t_reftable_obj_record_roundtrip(void)
    + {
    + 	uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 };
    + 	uint64_t till9[] = { 1, 2, 3, 4, 500, 600, 700, 800, 9000 };
     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
      		},
      	};
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip
      		struct string_view dest = {
      			.buf = buffer,
     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
    - 		test_copy(&in);
    + 		int n, m;
    + 		uint8_t extra;
    +
    +-		test_copy(&in);
    ++		t_copy(&in);
      		reftable_record_key(&in, &key);
      		n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ);
     -		EXPECT(n > 0);
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip
      		strbuf_release(&key);
      		reftable_record_release(&out);
      	}
    +@@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
    + 	strbuf_release(&scratch);
    + }
    +
    +-static void test_reftable_index_record_roundtrip(void)
    ++static void t_reftable_index_record_roundtrip(void)
    + {
    + 	struct reftable_record in = {
    + 		.type = BLOCK_TYPE_INDEX,
     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
    +
    + 	strbuf_addstr(&in.u.idx.last_key, "refs/heads/master");
      	reftable_record_key(&in, &key);
    - 	test_copy(&in);
    +-	test_copy(&in);
    ++	t_copy(&in);

     -	EXPECT(0 == strbuf_cmp(&key, &in.u.idx.last_key));
     +	check(!strbuf_cmp(&key, &in.u.idx.last_key));
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtr
     -	RUN_TEST(test_reftable_obj_record_roundtrip);
     -	RUN_TEST(test_reftable_index_record_roundtrip);
     -	return 0;
    -+	TEST(test_reftable_log_record_equal(), "reftable_log_record_equal 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");
    -+	TEST(test_key_roundtrip(), "reftable_encode_key and reftable_decode_key work");
    -+	TEST(test_reftable_obj_record_roundtrip(), "record operations work on obj record");
    -+	TEST(test_reftable_index_record_roundtrip(), "record operations work on index record");
    ++	TEST(t_reftable_log_record_equal(), "reftable_log_record_equal works");
    ++	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
    ++	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
    ++	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
    ++	TEST(t_key_roundtrip(), "reftable_encode_key and reftable_decode_key work");
    ++	TEST(t_reftable_obj_record_roundtrip(), "record operations work on obj record");
    ++	TEST(t_reftable_index_record_roundtrip(), "record operations work on index record");
     +
     +	return test_done();
      }
 2:  45ac972538 !  2:  1e8a229cbd t-reftable-record: add reftable_record_cmp() tests for log records
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

      ## t/unit-tests/t-reftable-record.c ##
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_ref_record_roundtrip(void)
      	strbuf_release(&scratch);
      }

    --static void test_reftable_log_record_equal(void)
    -+static void test_reftable_log_record_comparison(void)
    +-static void t_reftable_log_record_equal(void)
    ++static void t_reftable_log_record_comparison(void)
      {
     -	struct reftable_log_record in[2] = {
     +	struct reftable_record in[3] = {
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip
     +	check(!reftable_record_cmp(&in[0], &in[1]));
      }

    - static void test_reftable_log_record_roundtrip(void)
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
    + static void t_reftable_log_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_index_record_roundtrip(void)

      int cmd_main(int argc, const char *argv[])
      {
    --	TEST(test_reftable_log_record_equal(), "reftable_log_record_equal works");
    -+	TEST(test_reftable_log_record_comparison(), "comparison operations work on log record");
    - 	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");
    +-	TEST(t_reftable_log_record_equal(), "reftable_log_record_equal works");
    ++	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
    + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
    + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
    + 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
 3:  db76851f4b !  3:  f45611e493 t-reftable-record: add comparison tests for ref records
    @@ t/unit-tests/t-reftable-record.c: static void set_hash(uint8_t *h, int j)
      		h[i] = (j >> i) & 0xff;
      }

    -+static void test_reftable_ref_record_comparison(void)
    ++static void t_reftable_ref_record_comparison(void)
     +{
     +	struct reftable_record in[3] = {
     +		{
    @@ t/unit-tests/t-reftable-record.c: static void set_hash(uint8_t *h, int j)
     +	check(!reftable_record_cmp(&in[0], &in[1]));
     +}
     +
    - static void test_reftable_ref_record_roundtrip(void)
    + static void t_reftable_ref_record_roundtrip(void)
      {
      	struct strbuf scratch = STRBUF_INIT;
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_index_record_roundtrip(void)

      int cmd_main(int argc, const char *argv[])
      {
    -+	TEST(test_reftable_ref_record_comparison(), "comparison operations work on ref record");
    - 	TEST(test_reftable_log_record_comparison(), "comparison operations work on log record");
    - 	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(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
    + 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
    + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
    + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
 4:  78aff923c6 !  4:  28387b65e0 t-reftable-record: add comparison tests for index records
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

      ## t/unit-tests/t-reftable-record.c ##
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_obj_record_roundtrip(void)
      	strbuf_release(&scratch);
      }

    -+static void test_reftable_index_record_comparison(void)
    ++static void t_reftable_index_record_comparison(void)
     +{
     +	struct reftable_record in[3] = {
     +		{
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip
     +		reftable_record_release(&in[i]);
     +}
     +
    - static void test_reftable_index_record_roundtrip(void)
    + static void t_reftable_index_record_roundtrip(void)
      {
      	struct reftable_record in = {
     @@ t/unit-tests/t-reftable-record.c: int cmd_main(int argc, const char *argv[])
      {
    - 	TEST(test_reftable_ref_record_comparison(), "comparison operations work on ref record");
    - 	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_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");
    + 	TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
    + 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
    ++	TEST(t_reftable_index_record_comparison(), "comparison operations work on index record");
    + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
    + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
    + 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
 5:  b0b3c98042 !  5:  6349ce15f4 t-reftable-record: add comparison tests for obj records
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

      ## t/unit-tests/t-reftable-record.c ##
    -@@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_key_roundtrip(void)
      	strbuf_release(&roundtrip);
      }

    -+static void test_reftable_obj_record_comparison(void)
    ++static void t_reftable_obj_record_comparison(void)
     +{
     +
     +	uint8_t id_bytes[] = { 0, 1, 2, 3, 4, 5, 6 };
    @@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
     +	check(!reftable_record_cmp(&in[0], &in[1]));
     +}
     +
    - static void test_reftable_obj_record_roundtrip(void)
    + static void t_reftable_obj_record_roundtrip(void)
      {
      	uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 };
     @@ t/unit-tests/t-reftable-record.c: int cmd_main(int argc, const char *argv[])
    - 	TEST(test_reftable_ref_record_comparison(), "comparison operations work on ref record");
    - 	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_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");
    + 	TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
    + 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
    + 	TEST(t_reftable_index_record_comparison(), "comparison operations work on index record");
    ++	TEST(t_reftable_obj_record_comparison(), "comparison operations work on obj record");
    + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
    + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
    + 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
 6:  5e6b004216 !  6:  9202c783b9 t-reftable-record: add ref tests for reftable_record_is_deletion()
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

      ## t/unit-tests/t-reftable-record.c ##
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_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,
    @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip
      		};
      		struct reftable_record out = { .type = BLOCK_TYPE_REF };
      		struct strbuf key = STRBUF_INIT;
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
    - 		test_copy(&in);
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_ref_record_roundtrip(void)
    + 		t_copy(&in);

      		check_int(reftable_record_val_type(&in), ==, i);
     +		check_int(reftable_record_is_deletion(&in), ==, i == REFTABLE_REF_DELETION);
 7:  a68be88ccb !  7:  4632a00e15 t-reftable-record: add log tests for reftable_record_is_deletion()
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

      ## t/unit-tests/t-reftable-record.c ##
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_log_record_roundtrip(void)
      	set_hash(in[2].value.update.new_hash, 3);
      	set_hash(in[2].value.update.old_hash, 4);

 8:  02516add15 <  -:  ---------- t-reftable-record: add obj tests for reftable_record_is_deletion()
 -:  ---------- >  8:  3826ed5ef3 t-reftable-record: add obj tests for reftable_record_is_deletion()
 9:  541f9811d3 !  9:  1fba6d500c t-reftable-record: add index tests for reftable_record_is_deletion()
    @@ Commit message
         Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

      ## t/unit-tests/t-reftable-record.c ##
    -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
    +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_index_record_roundtrip(void)
      	reftable_record_key(&in, &key);
    - 	test_copy(&in);
    + 	t_copy(&in);

     +	check(!reftable_record_is_deletion(&in));
      	check(!strbuf_cmp(&key, &in.u.idx.last_key));
10:  c2aff283b1 <  -:  ---------- t-reftable-record: add tests for reftable_ref_record_compare_name()
11:  7bdfca3744 <  -:  ---------- t-reftable-record: add tests for reftable_log_record_compare_key()
 -:  ---------- > 10:  3511e36c18 t-reftable-record: add tests for reftable_ref_record_compare_name()
 -:  ---------- > 11:  97e5cbaeaf t-reftable-record: add tests for reftable_log_record_compare_key()

Comments

Karthik Nayak July 2, 2024, 10:55 a.m. UTC | #1
Hello,

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/record_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 v4:
> - Rename the tests to follow a 't_foo()' pattern instead
>   of 'test_foo()'
> - Use hard-coded test input in the 10th and 11th patch
>

Nice, I see my comments from the previous versions are addressed. It
looks good to me now!

> CI/PR: https://github.com/gitgitgadget/git/pull/1750
>
> Chandra Pratap (11):
> t: move reftable/record_test.c to the unit testing framework
> t-reftable-record: add reftable_record_cmp() tests for log records
> t-reftable-record: add comparison tests for ref records
> t-reftable-record: add comparison tests for index records
> t-reftable-record: add comparison tests for obj records
> t-reftable-record: add reftable_record_is_deletion() test for ref records
> t-reftable-record: add reftable_record_is_deletion() test for log records
> t-reftable-record: add reftable_record_is_deletion() test for obj records
> t-reftable-record: add reftable_record_is_deletion() test for index records
> t-reftable-record: add tests for reftable_ref_record_compare_name()
> t-reftable-record: add tests for reftable_log_record_compare_key()
>
> Makefile                         |   2 +-
> reftable/record_test.c           | 382 -------------------------
> t/helper/test-reftable.c         |   1 -
> t/unit-tests/t-reftable-record.c | 551 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 552 insertions(+), 384 deletions(-)
>
> Range-diff against v3:
>  1:  c88fb5bcfa !  1:  c210a0da8f t: move reftable/record_test.c to the unit testing framework
>     @@ Commit message
>          reftable/record.{c, h}. Migrate reftable/record_test.c to the
>          unit testing framework. Migration involves refactoring the tests
>          to use the unit testing framework instead of reftable's test
>     -    framework.
>     +    framework, and renaming the tests to fit unit-tests' naming scheme.
>     +
>          While at it, change the type of index variable 'i' to 'size_t'
>          from 'int'. This is because 'i' is used in comparison against
>          'ARRAY_SIZE(x)' which is of type 'size_t'.
>     @@ reftable/record_test.c => t/unit-tests/t-reftable-record.c
>       */
>
>      -#include "record.h"
>     --
>     ++#include "test-lib.h"
>     ++#include "reftable/constants.h"
>     ++#include "reftable/record.h"
>     +
>      -#include "system.h"
>      -#include "basics.h"
>      -#include "constants.h"
>      -#include "test_framework.h"
>      -#include "reftable-tests.h"
>     -+#include "test-lib.h"
>     -+#include "reftable/constants.h"
>     -+#include "reftable/record.h"
>     -
>     - static void test_copy(struct reftable_record *rec)
>     +-
>     +-static void test_copy(struct reftable_record *rec)
>     ++static void t_copy(struct reftable_record *rec)
>       {
>     + 	struct reftable_record copy;
>     + 	uint8_t typ;
>      @@ t/unit-tests/t-reftable-record.c: static void test_copy(struct reftable_record *rec)
>       	reftable_record_copy_from(&copy, rec, GIT_SHA1_RAWSZ);
>       	/* do it twice to catch memory leaks */
>     @@ t/unit-tests/t-reftable-record.c: static void test_copy(struct reftable_record *
>
>       	reftable_record_release(&copy);
>       }
>     +
>     +-static void test_varint_roundtrip(void)
>     ++static void t_varint_roundtrip(void)
>     + {
>     + 	uint64_t inputs[] = { 0,
>     + 			      1,
>      @@ t/unit-tests/t-reftable-record.c: static void test_varint_roundtrip(void)
>       			      4096,
>       			      ((uint64_t)1 << 63),
>     @@ t/unit-tests/t-reftable-record.c: static void test_varint_roundtrip(void)
>      -	}
>       }
>
>     - static void test_reftable_ref_record_roundtrip(void)
>     +-static void test_reftable_ref_record_roundtrip(void)
>     ++static void t_reftable_ref_record_roundtrip(void)
>       {
>       	struct strbuf scratch = STRBUF_INIT;
>      -	int i = 0;
>     @@ t/unit-tests/t-reftable-record.c: static void test_varint_roundtrip(void)
>       			.type = BLOCK_TYPE_REF,
>       		};
>      @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
>     + 		}
>     + 		in.u.ref.refname = xstrdup("refs/heads/master");
>
>     - 		test_copy(&in);
>     +-		test_copy(&in);
>     ++		t_copy(&in);
>
>      -		EXPECT(reftable_record_val_type(&in) == i);
>      +		check_int(reftable_record_val_type(&in), ==, i);
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip
>       						 GIT_SHA1_RAWSZ));
>       		reftable_record_release(&in);
>
>     +@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
>     + 	strbuf_release(&scratch);
>     + }
>     +
>     +-static void test_reftable_log_record_equal(void)
>     ++static void t_reftable_log_record_equal(void)
>     + {
>     + 	struct reftable_log_record in[2] = {
>     + 		{
>      @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_equal(void)
>       		}
>       	};
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_equal(voi
>       	reftable_log_record_release(&in[1]);
>       }
>
>     - static void test_reftable_log_record_roundtrip(void)
>     +-static void test_reftable_log_record_roundtrip(void)
>     ++static void t_reftable_log_record_roundtrip(void)
>       {
>      -	int i;
>       	struct reftable_log_record in[] = {
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip
>       		struct strbuf key = STRBUF_INIT;
>       		uint8_t buffer[1024] = { 0 };
>      @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip(void)
>     +
>     + 		rec.u.log = in[i];
>     +
>     +-		test_copy(&rec);
>     ++		t_copy(&rec);
>     +
>       		reftable_record_key(&rec, &key);
>
>       		n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ);
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip
>       						 GIT_SHA1_RAWSZ));
>       		reftable_log_record_release(&in[i]);
>       		strbuf_release(&key);
>     +@@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip(void)
>     + 	strbuf_release(&scratch);
>     + }
>     +
>     +-static void test_key_roundtrip(void)
>     ++static void t_key_roundtrip(void)
>     + {
>     + 	uint8_t buffer[1024] = { 0 };
>     + 	struct string_view dest = {
>      @@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
>       	strbuf_addstr(&key, "refs/tags/bla");
>       	extra = 6;
>     @@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
>
>       	strbuf_release(&last_key);
>       	strbuf_release(&key);
>     + 	strbuf_release(&roundtrip);
>     + }
>     +
>     +-static void test_reftable_obj_record_roundtrip(void)
>     ++static void t_reftable_obj_record_roundtrip(void)
>     + {
>     + 	uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 };
>     + 	uint64_t till9[] = { 1, 2, 3, 4, 500, 600, 700, 800, 9000 };
>      @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
>       		},
>       	};
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip
>       		struct string_view dest = {
>       			.buf = buffer,
>      @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
>     - 		test_copy(&in);
>     + 		int n, m;
>     + 		uint8_t extra;
>     +
>     +-		test_copy(&in);
>     ++		t_copy(&in);
>       		reftable_record_key(&in, &key);
>       		n = reftable_record_encode(&in, dest, GIT_SHA1_RAWSZ);
>      -		EXPECT(n > 0);
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip
>       		strbuf_release(&key);
>       		reftable_record_release(&out);
>       	}
>     +@@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
>     + 	strbuf_release(&scratch);
>     + }
>     +
>     +-static void test_reftable_index_record_roundtrip(void)
>     ++static void t_reftable_index_record_roundtrip(void)
>     + {
>     + 	struct reftable_record in = {
>     + 		.type = BLOCK_TYPE_INDEX,
>      @@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
>     +
>     + 	strbuf_addstr(&in.u.idx.last_key, "refs/heads/master");
>       	reftable_record_key(&in, &key);
>     - 	test_copy(&in);
>     +-	test_copy(&in);
>     ++	t_copy(&in);
>
>      -	EXPECT(0 == strbuf_cmp(&key, &in.u.idx.last_key));
>      +	check(!strbuf_cmp(&key, &in.u.idx.last_key));
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtr
>      -	RUN_TEST(test_reftable_obj_record_roundtrip);
>      -	RUN_TEST(test_reftable_index_record_roundtrip);
>      -	return 0;
>     -+	TEST(test_reftable_log_record_equal(), "reftable_log_record_equal 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");
>     -+	TEST(test_key_roundtrip(), "reftable_encode_key and reftable_decode_key work");
>     -+	TEST(test_reftable_obj_record_roundtrip(), "record operations work on obj record");
>     -+	TEST(test_reftable_index_record_roundtrip(), "record operations work on index record");
>     ++	TEST(t_reftable_log_record_equal(), "reftable_log_record_equal works");
>     ++	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
>     ++	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
>     ++	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
>     ++	TEST(t_key_roundtrip(), "reftable_encode_key and reftable_decode_key work");
>     ++	TEST(t_reftable_obj_record_roundtrip(), "record operations work on obj record");
>     ++	TEST(t_reftable_index_record_roundtrip(), "record operations work on index record");
>      +
>      +	return test_done();
>       }
>  2:  45ac972538 !  2:  1e8a229cbd t-reftable-record: add reftable_record_cmp() tests for log records
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-record.c ##
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_ref_record_roundtrip(void)
>       	strbuf_release(&scratch);
>       }
>
>     --static void test_reftable_log_record_equal(void)
>     -+static void test_reftable_log_record_comparison(void)
>     +-static void t_reftable_log_record_equal(void)
>     ++static void t_reftable_log_record_comparison(void)
>       {
>      -	struct reftable_log_record in[2] = {
>      +	struct reftable_record in[3] = {
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip
>      +	check(!reftable_record_cmp(&in[0], &in[1]));
>       }
>
>     - static void test_reftable_log_record_roundtrip(void)
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
>     + static void t_reftable_log_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_index_record_roundtrip(void)
>
>       int cmd_main(int argc, const char *argv[])
>       {
>     --	TEST(test_reftable_log_record_equal(), "reftable_log_record_equal works");
>     -+	TEST(test_reftable_log_record_comparison(), "comparison operations work on log record");
>     - 	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");
>     +-	TEST(t_reftable_log_record_equal(), "reftable_log_record_equal works");
>     ++	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
>     + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
>     + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
>     + 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
>  3:  db76851f4b !  3:  f45611e493 t-reftable-record: add comparison tests for ref records
>     @@ t/unit-tests/t-reftable-record.c: static void set_hash(uint8_t *h, int j)
>       		h[i] = (j >> i) & 0xff;
>       }
>
>     -+static void test_reftable_ref_record_comparison(void)
>     ++static void t_reftable_ref_record_comparison(void)
>      +{
>      +	struct reftable_record in[3] = {
>      +		{
>     @@ t/unit-tests/t-reftable-record.c: static void set_hash(uint8_t *h, int j)
>      +	check(!reftable_record_cmp(&in[0], &in[1]));
>      +}
>      +
>     - static void test_reftable_ref_record_roundtrip(void)
>     + static void t_reftable_ref_record_roundtrip(void)
>       {
>       	struct strbuf scratch = STRBUF_INIT;
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_index_record_roundtrip(void)
>
>       int cmd_main(int argc, const char *argv[])
>       {
>     -+	TEST(test_reftable_ref_record_comparison(), "comparison operations work on ref record");
>     - 	TEST(test_reftable_log_record_comparison(), "comparison operations work on log record");
>     - 	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(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
>     + 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
>     + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
>     + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
>  4:  78aff923c6 !  4:  28387b65e0 t-reftable-record: add comparison tests for index records
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-record.c ##
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_obj_record_roundtrip(void)
>       	strbuf_release(&scratch);
>       }
>
>     -+static void test_reftable_index_record_comparison(void)
>     ++static void t_reftable_index_record_comparison(void)
>      +{
>      +	struct reftable_record in[3] = {
>      +		{
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_obj_record_roundtrip
>      +		reftable_record_release(&in[i]);
>      +}
>      +
>     - static void test_reftable_index_record_roundtrip(void)
>     + static void t_reftable_index_record_roundtrip(void)
>       {
>       	struct reftable_record in = {
>      @@ t/unit-tests/t-reftable-record.c: int cmd_main(int argc, const char *argv[])
>       {
>     - 	TEST(test_reftable_ref_record_comparison(), "comparison operations work on ref record");
>     - 	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_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");
>     + 	TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
>     + 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
>     ++	TEST(t_reftable_index_record_comparison(), "comparison operations work on index record");
>     + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
>     + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
>     + 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
>  5:  b0b3c98042 !  5:  6349ce15f4 t-reftable-record: add comparison tests for obj records
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-record.c ##
>     -@@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_key_roundtrip(void)
>       	strbuf_release(&roundtrip);
>       }
>
>     -+static void test_reftable_obj_record_comparison(void)
>     ++static void t_reftable_obj_record_comparison(void)
>      +{
>      +
>      +	uint8_t id_bytes[] = { 0, 1, 2, 3, 4, 5, 6 };
>     @@ t/unit-tests/t-reftable-record.c: static void test_key_roundtrip(void)
>      +	check(!reftable_record_cmp(&in[0], &in[1]));
>      +}
>      +
>     - static void test_reftable_obj_record_roundtrip(void)
>     + static void t_reftable_obj_record_roundtrip(void)
>       {
>       	uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 };
>      @@ t/unit-tests/t-reftable-record.c: int cmd_main(int argc, const char *argv[])
>     - 	TEST(test_reftable_ref_record_comparison(), "comparison operations work on ref record");
>     - 	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_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");
>     + 	TEST(t_reftable_ref_record_comparison(), "comparison operations work on ref record");
>     + 	TEST(t_reftable_log_record_comparison(), "comparison operations work on log record");
>     + 	TEST(t_reftable_index_record_comparison(), "comparison operations work on index record");
>     ++	TEST(t_reftable_obj_record_comparison(), "comparison operations work on obj record");
>     + 	TEST(t_reftable_log_record_roundtrip(), "record operations work on log record");
>     + 	TEST(t_reftable_ref_record_roundtrip(), "record operations work on ref record");
>     + 	TEST(t_varint_roundtrip(), "put_var_int and get_var_int work");
>  6:  5e6b004216 !  6:  9202c783b9 t-reftable-record: add ref tests for reftable_record_is_deletion()
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-record.c ##
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_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,
>     @@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip
>       		};
>       		struct reftable_record out = { .type = BLOCK_TYPE_REF };
>       		struct strbuf key = STRBUF_INIT;
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_ref_record_roundtrip(void)
>     - 		test_copy(&in);
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_ref_record_roundtrip(void)
>     + 		t_copy(&in);
>
>       		check_int(reftable_record_val_type(&in), ==, i);
>      +		check_int(reftable_record_is_deletion(&in), ==, i == REFTABLE_REF_DELETION);
>  7:  a68be88ccb !  7:  4632a00e15 t-reftable-record: add log tests for reftable_record_is_deletion()
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-record.c ##
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_log_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_log_record_roundtrip(void)
>       	set_hash(in[2].value.update.new_hash, 3);
>       	set_hash(in[2].value.update.old_hash, 4);
>
>  8:  02516add15 <  -:  ---------- t-reftable-record: add obj tests for reftable_record_is_deletion()
>  -:  ---------- >  8:  3826ed5ef3 t-reftable-record: add obj tests for reftable_record_is_deletion()
>  9:  541f9811d3 !  9:  1fba6d500c t-reftable-record: add index tests for reftable_record_is_deletion()
>     @@ Commit message
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
>       ## t/unit-tests/t-reftable-record.c ##
>     -@@ t/unit-tests/t-reftable-record.c: static void test_reftable_index_record_roundtrip(void)
>     +@@ t/unit-tests/t-reftable-record.c: static void t_reftable_index_record_roundtrip(void)
>       	reftable_record_key(&in, &key);
>     - 	test_copy(&in);
>     + 	t_copy(&in);
>
>      +	check(!reftable_record_is_deletion(&in));
>       	check(!strbuf_cmp(&key, &in.u.idx.last_key));
> 10:  c2aff283b1 <  -:  ---------- t-reftable-record: add tests for reftable_ref_record_compare_name()
> 11:  7bdfca3744 <  -:  ---------- t-reftable-record: add tests for reftable_log_record_compare_key()
>  -:  ---------- > 10:  3511e36c18 t-reftable-record: add tests for reftable_ref_record_compare_name()
>  -:  ---------- > 11:  97e5cbaeaf t-reftable-record: add tests for reftable_log_record_compare_key()
Junio C Hamano July 2, 2024, 3:14 p.m. UTC | #2
Karthik Nayak <karthik.188@gmail.com> writes:

> Hello,
>
> 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/record_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 v4:
>> - Rename the tests to follow a 't_foo()' pattern instead
>>   of 'test_foo()'
>> - Use hard-coded test input in the 10th and 11th patch
>>
>
> Nice, I see my comments from the previous versions are addressed. It
> looks good to me now!

Thanks, both.