| Message ID | patch-v4-2.6-d351075f0ab-20221219T101240Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 62f3a45bb49f9436f1cd754b02ac549b1f6514cf |
| Headers | show |
| Series | tests: fix ignored & hidden exit codes | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Fix code added in b319ef70a94 (Add a small patch-mode testing library, > 2009-08-13) to use &&-chaining. > > This avoids losing both the exit code of a "git" and the "cat" > processes. > > This fixes cases where we'd have e.g. missed memory leaks under > SANITIZE=leak, this code doesn't leak now as far as I can tell, but I > discovered it while looking at leaks in related code. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/lib-patch-mode.sh | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) Looks quite sensible. > diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh > index cfd76bf987b..89ca1f78055 100644 > --- a/t/lib-patch-mode.sh > +++ b/t/lib-patch-mode.sh > @@ -29,8 +29,12 @@ set_and_save_state () { > > # verify_state <path> <expected-worktree-content> <expected-index-content> > verify_state () { > - test "$(cat "$1")" = "$2" && > - test "$(git show :"$1")" = "$3" > + echo "$2" >expect && > + test_cmp expect "$1" && > + > + echo "$3" >expect && > + git show :"$1" >actual && > + test_cmp expect actual > } > > # verify_saved_state <path> > @@ -46,5 +50,6 @@ save_head () { > } > > verify_saved_head () { > - test "$(cat _head)" = "$(git rev-parse HEAD)" > + git rev-parse HEAD >actual && > + test_cmp _head actual > }
Hi Ævar On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote: > Fix code added in b319ef70a94 (Add a small patch-mode testing library, > 2009-08-13) to use &&-chaining. > > This avoids losing both the exit code of a "git" and the "cat" > processes. > > This fixes cases where we'd have e.g. missed memory leaks under > SANITIZE=leak, this code doesn't leak now as far as I can tell, but I > discovered it while looking at leaks in related code. > [...] > # verify_saved_state <path> > @@ -46,5 +50,6 @@ save_head () { > } > > verify_saved_head () { > - test "$(cat _head)" = "$(git rev-parse HEAD)" > + git rev-parse HEAD >actual && > + test_cmp _head actual Aren't these two lines are re-implementing test_cmp_rev()? Best Wishes Phillip
On Tue, Dec 27 2022, Phillip Wood wrote: > Hi Ævar > > On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote: >> Fix code added in b319ef70a94 (Add a small patch-mode testing library, >> 2009-08-13) to use &&-chaining. >> This avoids losing both the exit code of a "git" and the "cat" >> processes. >> This fixes cases where we'd have e.g. missed memory leaks under >> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I >> discovered it while looking at leaks in related code. >> [...] # verify_saved_state <path> >> @@ -46,5 +50,6 @@ save_head () { >> } >> verify_saved_head () { >> - test "$(cat _head)" = "$(git rev-parse HEAD)" >> + git rev-parse HEAD >actual && >> + test_cmp _head actual > > Aren't these two lines are re-implementing test_cmp_rev()? It does --verify, and this does not. Could we use it? Yes, but I wanted to narrowly focus on just fixing the lost exit codes in this series. Once you start to untangle "save_head" and "verify_saved_head" you'll see that whether we narrowly used a helper here or not isn't the only thing we could improve. But such an improvement would make use use --verify, and we'd then want to use that "--verify" for the earlier saved_head too, etc.
diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh index cfd76bf987b..89ca1f78055 100644 --- a/t/lib-patch-mode.sh +++ b/t/lib-patch-mode.sh @@ -29,8 +29,12 @@ set_and_save_state () { # verify_state <path> <expected-worktree-content> <expected-index-content> verify_state () { - test "$(cat "$1")" = "$2" && - test "$(git show :"$1")" = "$3" + echo "$2" >expect && + test_cmp expect "$1" && + + echo "$3" >expect && + git show :"$1" >actual && + test_cmp expect actual } # verify_saved_state <path> @@ -46,5 +50,6 @@ save_head () { } verify_saved_head () { - test "$(cat _head)" = "$(git rev-parse HEAD)" + git rev-parse HEAD >actual && + test_cmp _head actual }
Fix code added in b319ef70a94 (Add a small patch-mode testing library, 2009-08-13) to use &&-chaining. This avoids losing both the exit code of a "git" and the "cat" processes. This fixes cases where we'd have e.g. missed memory leaks under SANITIZE=leak, this code doesn't leak now as far as I can tell, but I discovered it while looking at leaks in related code. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/lib-patch-mode.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)