Re: [PATCH v4 5/7] tests: don't swallow Git errors upstream of pipes

[Matthew DeVore]

I just realized that the changes to t9101 should actually be part of
the next patch (6/7), not this one. I've fixed that for the next
re-roll.

Re: [PATCH v4 5/7] tests: don't swallow Git errors upstream of pipes

[Matthew DeVore]

On Fri, Oct 5, 2018 at 9:48 AM Junio C Hamano  wrote:
>
> Hopefully this is not a blind mechanical patch, as introduction of
> unexpected temporary files in the working tree could interfere with
> later tests (e.g. they may expect exact set of untracked files, and
> these new temporary files would b unexpected additions).
>
> Thanks.

I reviewed the files modified in this patch. For the svn one, there
are other scratch files being created in the cwd before mine, so my
additions should be fine, I admit I don't fully understand the
commands being run in that test...after the temp files are created, I
see git svn proplist and git svn propget being executed... the public
documentation for them does not seem to contradict my assumption.
(http://svnbook.red-bean.com/en/1.6/svn.ref.svn.c.propget.html)

As for the other test files in this patch, all git invocations are
either "git clone" or "git init " or "git -C ..." which means
none of them care about extra files in the cwd. After I searched for
Git commands I read through the files somewhat briskly and looked for
any globs or suspicious invocations of awk, grep, or other commands
(we really do use a limited number of commands in the tests I saw :)
and did not find anything that depended on existence or non-existence
of unnamed files.

Re: [PATCH v4 5/7] tests: don't swallow Git errors upstream of pipes

[Junio C Hamano]

Matthew DeVore  writes:

> Some pipes in tests lose the exit code of git processes, which can mask
> unexpected behavior like crashes. Split these pipes up so that git
> commands are only at the end of pipes rather than the beginning or
> middle.
>
> The violations fixed in this patch were found in the process of fixing
> pipe placement in a prior patch.

Hopefully this is not a blind mechanical patch, as introduction of
unexpected temporary files in the working tree could interfere with
later tests (e.g. they may expect exact set of untracked files, and
these new temporary files would b unexpected additions).

Thanks.

[PATCH v4 5/7] tests: don't swallow Git errors upstream of pipes

[Matthew DeVore]

Some pipes in tests lose the exit code of git processes, which can mask
unexpected behavior like crashes. Split these pipes up so that git
commands are only at the end of pipes rather than the beginning or
middle.

The violations fixed in this patch were found in the process of fixing
pipe placement in a prior patch.

Signed-off-by: Matthew DeVore 
---
 t/t5317-pack-objects-filter-objects.sh | 156 +
 t/t5616-partial-clone.sh   |  14 ++-
 t/t6112-rev-list-filters-objects.sh| 103 
 t/t9101-git-svn-props.sh   |   3 +-
 4 files changed, 143 insertions(+), 133 deletions(-)

diff --git a/t/t5317-pack-objects-filter-objects.sh 
b/t/t5317-pack-objects-filter-objects.sh
index c093eb891..2e718f0bd 100755
--- a/t/t5317-pack-objects-filter-objects.sh
+++ b/t/t5317-pack-objects-filter-objects.sh
@@ -20,8 +20,9 @@ test_expect_success 'setup r1' '
 '
 
 test_expect_success 'verify blob count in normal packfile' '
-   git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 |
-   awk -f print_2.awk |
+   git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \
+   >ls_files_result &&
+   awk -f print_2.awk ls_files_result |
sort >expected &&
 
git -C r1 pack-objects --rev --stdout >all.pack <<-EOF &&
@@ -29,8 +30,8 @@ test_expect_success 'verify blob count in normal packfile' '
EOF
git -C r1 index-pack ../all.pack &&
 
-   git -C r1 verify-pack -v ../all.pack |
-   grep blob |
+   git -C r1 verify-pack -v ../all.pack >verify_result &&
+   grep blob verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -43,8 +44,8 @@ test_expect_success 'verify blob:none packfile has no blobs' '
EOF
git -C r1 index-pack ../filter.pack &&
 
-   git -C r1 verify-pack -v ../filter.pack |
-   grep blob |
+   git -C r1 verify-pack -v ../filter.pack >verify_result &&
+   grep blob verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -53,13 +54,13 @@ test_expect_success 'verify blob:none packfile has no 
blobs' '
 '
 
 test_expect_success 'verify normal and blob:none packfiles have same 
commits/trees' '
-   git -C r1 verify-pack -v ../all.pack |
-   grep -E "commit|tree" |
+   git -C r1 verify-pack -v ../all.pack >verify_result &&
+   grep -E "commit|tree" verify_result |
awk -f print_1.awk |
sort >expected &&
 
-   git -C r1 verify-pack -v ../filter.pack |
-   grep -E "commit|tree" |
+   git -C r1 verify-pack -v ../filter.pack >verify_result &&
+   grep -E "commit|tree" verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -82,8 +83,8 @@ test_expect_success 'setup r2' '
 '
 
 test_expect_success 'verify blob count in normal packfile' '
-   git -C r2 ls-files -s large.1000 large.1 |
-   awk -f print_2.awk |
+   git -C r2 ls-files -s large.1000 large.1 >ls_files_result &&
+   awk -f print_2.awk ls_files_result |
sort >expected &&
 
git -C r2 pack-objects --rev --stdout >all.pack <<-EOF &&
@@ -91,8 +92,8 @@ test_expect_success 'verify blob count in normal packfile' '
EOF
git -C r2 index-pack ../all.pack &&
 
-   git -C r2 verify-pack -v ../all.pack |
-   grep blob |
+   git -C r2 verify-pack -v ../all.pack >verify_result &&
+   grep blob verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -105,8 +106,8 @@ test_expect_success 'verify blob:limit=500 omits all blobs' 
'
EOF
git -C r2 index-pack ../filter.pack &&
 
-   git -C r2 verify-pack -v ../filter.pack |
-   grep blob |
+   git -C r2 verify-pack -v ../filter.pack >verify_result &&
+   grep blob verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -120,8 +121,8 @@ test_expect_success 'verify blob:limit=1000' '
EOF
git -C r2 index-pack ../filter.pack &&
 
-   git -C r2 verify-pack -v ../filter.pack |
-   grep blob |
+   git -C r2 verify-pack -v ../filter.pack >verify_result &&
+   grep blob verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -130,8 +131,8 @@ test_expect_success 'verify blob:limit=1000' '
 '
 
 test_expect_success 'verify blob:limit=1001' '
-   git -C r2 ls-files -s large.1000 |
-   awk -f print_2.awk |
+   git -C r2 ls-files -s large.1000 >ls_files_result &&
+   awk -f print_2.awk ls_files_result |
sort >expected &&
 
git -C r2 pack-objects --rev --stdout --filter=blob:limit=1001 
>filter.pack <<-EOF &&
@@ -139,8 +140,8 @@ test_expect_success 'verify blob:limit=1001' '
EOF
git -C r2 index-pack ../filter.pack &&
 
-   git -C r2 verify-pack -v ../filter.pack |
-   grep blob |
+   git -C r2 verify-pack -v ../filter.pack >verify_result &&
+   grep blob verify_result |
awk -f print_1.awk |
sort >observed &&
 
@@ -148