| Message ID | 20240804141105.4268-1-chandrapratap3519@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | t: port reftable/tree_test.c to the unit testing framework | expand |
On Sun, Aug 04, 2024 at 07:36:44PM +0530, Chandra Pratap wrote: > The reftable library comes with self tests, which are exercised > as part of the usual end-to-end tests and are designed to > observe the end-user visible effects of Git commands. What it > exercises, however, is a better match for the unit-testing > framework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests', > 2023-12-09), which is designed to observe how low level > implementation details, at the level of sequences of individual > function calls, behave. > > Hence, port reftable/tree_test.c to the unit testing framework and > improve upon the ported test. The first patch in the series is > preparatory cleanup, the second patch 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> Only a single change compared to v6, addressing the only feedback on that version. So this looks good to me, thanks! Patrick
Patrick Steinhardt <ps@pks.im> writes: > On Sun, Aug 04, 2024 at 07:36:44PM +0530, Chandra Pratap wrote: >> The reftable library comes with self tests, which are exercised >> as part of the usual end-to-end tests and are designed to >> observe the end-user visible effects of Git commands. What it >> exercises, however, is a better match for the unit-testing >> framework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests', >> 2023-12-09), which is designed to observe how low level >> implementation details, at the level of sequences of individual >> function calls, behave. >> >> Hence, port reftable/tree_test.c to the unit testing framework and >> improve upon the ported test. The first patch in the series is >> preparatory cleanup, the second patch 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> > > Only a single change compared to v6, addressing the only feedback on > that version. So this looks good to me, thanks! FWIW, I didn't have other feedback not because I found the rest perfect, but because I didn't read the series myself carefully, hoping others are sharing the burden. Thanks.
On Mon, Aug 05, 2024 at 08:53:09AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > On Sun, Aug 04, 2024 at 07:36:44PM +0530, Chandra Pratap wrote: > >> The reftable library comes with self tests, which are exercised > >> as part of the usual end-to-end tests and are designed to > >> observe the end-user visible effects of Git commands. What it > >> exercises, however, is a better match for the unit-testing > >> framework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests', > >> 2023-12-09), which is designed to observe how low level > >> implementation details, at the level of sequences of individual > >> function calls, behave. > >> > >> Hence, port reftable/tree_test.c to the unit testing framework and > >> improve upon the ported test. The first patch in the series is > >> preparatory cleanup, the second patch 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> > > > > Only a single change compared to v6, addressing the only feedback on > > that version. So this looks good to me, thanks! > > FWIW, I didn't have other feedback not because I found the rest > perfect, but because I didn't read the series myself carefully, > hoping others are sharing the burden. Oh, yes. I didn't mean to say that I relied on your feedback being addressed exclusively. I already reviewed v5/v6 of this patch series and found it to be good, and given that there was only a single change proposed by you that I'm happy with it translates into me being in favor of v7, as well. Patrick
Patrick Steinhardt <ps@pks.im> writes: > Oh, yes. I didn't mean to say that I relied on your feedback being > addressed exclusively. I already reviewed v5/v6 of this patch series and > found it to be good, and given that there was only a single change > proposed by you that I'm happy with it translates into me being in favor > of v7, as well. Yes, after sending the message you are responding to, I went back to the original thread and figured that much---the topic is marked for 'next'. Thanks for your help.
The reftable library comes with self tests, which are exercised as part of the usual end-to-end tests and are designed to observe the end-user visible effects of Git commands. What it exercises, however, is a better match for the unit-testing framework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests', 2023-12-09), which is designed to observe how low level implementation details, at the level of sequences of individual function calls, behave. Hence, port reftable/tree_test.c to the unit testing framework and improve upon the ported test. The first patch in the series is preparatory cleanup, the second patch 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 v7: - Fix style issues in a comment introduced in patch 3 of the previous series CI/PR: https://github.com/gitgitgadget/git/pull/1740 Chandra Pratap(5): reftable: remove unnecessary curly braces in reftable/tree.c t: move reftable/tree_test.c to the unit testing framework t-reftable-tree: split test_tree() into two sub-test t-reftable-tree: add test for non-existent key t-reftable-tree: improve the test for infix_walk() Makefile | 2 +- reftable/reftable-tests.h | 1 - reftable/tree.c | 15 +++------- reftable/tree_test.c | 60 ---------------------- t/helper/test-reftable.c | 1 - t/unit-tests/t-reftable-tree.c | 83 +++++++++++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+), 73 deletions(-) Range-diff against v6: 1: d4a45e602c = 1: d738bf57e2 reftable: remove unnecessary curly braces in reftable/tree.c 2: 9148e740e8 ! 2: f090ace685 t: move reftable/tree_test.c to the unit testing framework @@ t/unit-tests/t-reftable-tree.c (new) + size_t i = 1; + struct curry c = { 0 }; + -+ /* pseudo-randomly insert the pointers for elements between -+ * values[1] and values[10] (included) in the tree. ++ /* ++ * Pseudo-randomly insert the pointers for elements between ++ * values[1] and values[10] (inclusive) in the tree. + */ + do { + nodes[i] = tree_search(&values[i], &root, &t_compare, 1); 3: f73ad11238 ! 3: 22256e77b3 t-reftable-tree: split test_tree() into two sub-test functions @@ t/unit-tests/t-reftable-tree.c: static void check_increasing(void *arg, void *ke size_t i = 1; - struct curry c = { 0 }; -- /* pseudo-randomly insert the pointers for elements between -- * values[1] and values[10] (included) in the tree. -+ /* Pseudo-randomly insert the pointers for elements between -+ * values[1] and values[10] (inclusive) in the tree. - */ - do { - nodes[i] = tree_search(&values[i], &root, &t_compare, 1); + /* + * Pseudo-randomly insert the pointers for elements between @@ t/unit-tests/t-reftable-tree.c: static void t_tree(void) check_pointer_eq(nodes[i], tree_search(&values[i], &root, &t_compare, 0)); } 4: edb02d2e84 = 4: 0d04daad28 t-reftable-tree: add test for non-existent key 5: 6aecd4e374 = 5: 80d4aa2a66 t-reftable-tree: improve the test for infix_walk()