| Message ID | 20240621115708.3626-11-chandrapratap3519@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] t: move reftable/record_test.c to the unit testing framework | expand |
Chandra Pratap <chandrapratap3519@gmail.com> writes: > reftable_ref_record_compare_name() is a function defined by > reftable/record.{c, h} and is used to compare the refname of two > ref records when sorting multiple ref records using 'qsort'. > In the current testing setup, this function is left unexercised. > Add a testing function for the same. > > 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 | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c > index 99ebfafe0b..b949617c88 100644 > --- a/t/unit-tests/t-reftable-record.c > +++ b/t/unit-tests/t-reftable-record.c > @@ -101,6 +101,23 @@ static void test_reftable_ref_record_comparison(void) > reftable_record_release(&in[i]); > } > > +static void test_reftable_ref_record_compare_name(void) > +{ > + struct reftable_ref_record recs[14] = { 0 }; > + size_t N = ARRAY_SIZE(recs), i; > + > + for (i = 0; i < N; i++) > + recs[i].refname = xstrfmt("%02"PRIuMAX, (uintmax_t)i); > + > + QSORT(recs, N, reftable_ref_record_compare_name); > + > + for (i = 1; i < N; i++) > + check(reftable_ref_record_compare_name(&recs[i - 1], &recs[i]) < 0); > + I understand the intention, but using a function to help sort strings and then using validating the same function with those strings doesn't validate the functionality of the function. I would have preferred to see some hardcoded strings and simply comparison between them. Also comparison's where `reftable_ref_record_compare_name` returns '0' and '> 0' values. > + for (i = 0; i < N; i++) > + reftable_ref_record_release(&recs[i]); > +} > + > static void test_reftable_ref_record_roundtrip(void) > { > struct strbuf scratch = STRBUF_INIT; > @@ -501,6 +518,7 @@ int cmd_main(int argc, const char *argv[]) > 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_ref_record_compare_name(), "reftable_ref_record_compare_name 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"); > -- > 2.45.2.404.g9eaef5822c
On Tue, 25 Jun 2024 at 14:56, Karthik Nayak <karthik.188@gmail.com> wrote: > > Chandra Pratap <chandrapratap3519@gmail.com> writes: > > > reftable_ref_record_compare_name() is a function defined by > > reftable/record.{c, h} and is used to compare the refname of two > > ref records when sorting multiple ref records using 'qsort'. > > In the current testing setup, this function is left unexercised. > > Add a testing function for the same. > > > > 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 | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c > > index 99ebfafe0b..b949617c88 100644 > > --- a/t/unit-tests/t-reftable-record.c > > +++ b/t/unit-tests/t-reftable-record.c > > @@ -101,6 +101,23 @@ static void test_reftable_ref_record_comparison(void) > > reftable_record_release(&in[i]); > > } > > > > +static void test_reftable_ref_record_compare_name(void) > > +{ > > + struct reftable_ref_record recs[14] = { 0 }; > > + size_t N = ARRAY_SIZE(recs), i; > > + > > + for (i = 0; i < N; i++) > > + recs[i].refname = xstrfmt("%02"PRIuMAX, (uintmax_t)i); > > + > > + QSORT(recs, N, reftable_ref_record_compare_name); > > + > > + for (i = 1; i < N; i++) > > + check(reftable_ref_record_compare_name(&recs[i - 1], &recs[i]) < 0); > > + > > I understand the intention, but using a function to help sort strings > and then using validating the same function with those strings doesn't > validate the functionality of the function. True. I'll modify this to use strcmp() instead. > I would have preferred to see some hardcoded strings and simply > comparison between them. Also comparison's where > `reftable_ref_record_compare_name` returns '0' and '> 0' values. The only use case for this function is as the comparison function for QSORT(), so I wanted to design a test close to that use case. Nevertheless, I'll add tests for '== 0' and '> 0' cases. > > + for (i = 0; i < N; i++) > > + reftable_ref_record_release(&recs[i]); > > +} > > + > > static void test_reftable_ref_record_roundtrip(void) > > { > > struct strbuf scratch = STRBUF_INIT; > > @@ -501,6 +518,7 @@ int cmd_main(int argc, const char *argv[]) > > 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_ref_record_compare_name(), "reftable_ref_record_compare_name 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"); > > -- > > 2.45.2.404.g9eaef5822c
diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c index 99ebfafe0b..b949617c88 100644 --- a/t/unit-tests/t-reftable-record.c +++ b/t/unit-tests/t-reftable-record.c @@ -101,6 +101,23 @@ static void test_reftable_ref_record_comparison(void) reftable_record_release(&in[i]); } +static void test_reftable_ref_record_compare_name(void) +{ + struct reftable_ref_record recs[14] = { 0 }; + size_t N = ARRAY_SIZE(recs), i; + + for (i = 0; i < N; i++) + recs[i].refname = xstrfmt("%02"PRIuMAX, (uintmax_t)i); + + QSORT(recs, N, reftable_ref_record_compare_name); + + for (i = 1; i < N; i++) + check(reftable_ref_record_compare_name(&recs[i - 1], &recs[i]) < 0); + + for (i = 0; i < N; i++) + reftable_ref_record_release(&recs[i]); +} + static void test_reftable_ref_record_roundtrip(void) { struct strbuf scratch = STRBUF_INIT; @@ -501,6 +518,7 @@ int cmd_main(int argc, const char *argv[]) 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_ref_record_compare_name(), "reftable_ref_record_compare_name 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");
reftable_ref_record_compare_name() is a function defined by reftable/record.{c, h} and is used to compare the refname of two ref records when sorting multiple ref records using 'qsort'. In the current testing setup, this function is left unexercised. Add a testing function for the same. 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 | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)