Hello,
Chandra Pratap <chandrapratap3519@xxxxxxxxx> 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@xxxxxx>
> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> Signed-off-by: Chandra Pratap <chandrapratap3519@xxxxxxxxx>
>
> ---
> 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(©, 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(©);
> }
> +
> +-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@xxxxxxxxx>
>
> ## 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@xxxxxxxxx>
>
> ## 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@xxxxxxxxx>
>
> ## 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@xxxxxxxxx>
>
> ## 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@xxxxxxxxx>
>
> ## 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@xxxxxxxxx>
>
> ## 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()
Attachment:
signature.asc
Description: PGP signature
