| Message ID | 20200125230035.136348-9-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256 test fixes, part 8 | expand |
Hi brian, On Sat, 25 Jan 2020, brian m. carlson wrote: > This test produces a large number of diff formats and compares the > output with test files that have content specific to SHA-1. Since we are > more interested in the format of the diffs, and not their specific > values, which are tested elsewhere, add a function which uses sed to > transform these specific object IDs into generic ones of the right size, > which we can then compare. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > t/t4013-diff-various.sh | 44 +++++++++++++++++++++++++++++++++++------ > 1 file changed, 38 insertions(+), 6 deletions(-) > > diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > index 5ac94b390d..6f5f05c3a8 100755 > --- a/t/t4013-diff-various.sh > +++ b/t/t4013-diff-various.sh > @@ -120,6 +120,30 @@ test_expect_success setup ' > +*++ [initial] Initial > EOF > > +process_diffs () { > + x04="[0-9a-f][0-9a-f][0-9a-f][0-9a-f]" && > + x07="$_x05[0-9a-f][0-9a-f]" && Any reason not to stay with the convention, i.e. using `_x04` and `_x07` here (with leading underscores)? > + sed -e "s/$OID_REGEX/$ZERO_OID/g" \ > + -e "s/From $_x40 /From $ZERO_OID /" \ > + -e "s/from $_x40)/from $ZERO_OID)/" \ > + -e "s/commit $_x40\$/commit $ZERO_OID/" \ > + -e "s/commit $_x40 (/commit $ZERO_OID (/" \ > + -e "s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/" \ > + -e "s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /" \ > + -e "s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/" \ > + -e "s/^$_x40 /$ZERO_OID /" \ > + -e "s/^$_x40$/$ZERO_OID/" \ > + -e "s/$x07\.\.$x07/fffffff..fffffff/g" \ > + -e "s/$x07,$x07\.\.$x07/fffffff,fffffff..fffffff/g" \ > + -e "s/$x07 $x07 $x07/fffffff fffffff fffffff/g" \ > + -e "s/$x07 $x07 /fffffff fffffff /g" \ > + -e "s/Merge: $x07 $x07/Merge: fffffff fffffff/g" \ > + -e "s/$x07\.\.\./fffffff.../g" \ > + -e "s/ $x04\.\.\./ ffff.../g" \ > + -e "s/ $x04/ ffff/g" \ > + "$1" > +} > + > V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g') > while read magic cmd > do > @@ -158,13 +182,15 @@ do > } >"$actual" && > if test -f "$expect" > then > + process_diffs "$actual" >actual && > + process_diffs "$expect" >expect && > case $cmd in > *format-patch* | *-stat*) > - test_i18ncmp "$expect" "$actual";; > + test_i18ncmp expect actual;; > *) > - test_cmp "$expect" "$actual";; > + test_cmp expect actual;; > esac && > - rm -f "$actual" > + rm -f "$actual" actual expect > else > # this is to help developing new tests. > cp "$actual" "$expect" > @@ -383,16 +409,22 @@ test_expect_success 'log -S requires an argument' ' > test_expect_success 'diff --cached on unborn branch' ' > echo ref: refs/heads/unborn >.git/HEAD && > git diff --cached >result && > - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached" result > + process_diffs result >actual && > + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached" >expected && I was about to suggest letting `process_diffs` work in-place, but this line makes that idea moot. Another idea I had was to implement a `test_cmp_diff` that processes the diffs and then compares them, but I guess that would be _less_ concise than this patch. Looks good, Dscho > + test_cmp expected actual > ' > > test_expect_success 'diff --cached -- file on unborn branch' ' > git diff --cached -- file0 >result && > - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result > + process_diffs result >actual && > + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" >expected && > + test_cmp expected actual > ' > test_expect_success 'diff --line-prefix with spaces' ' > git diff --line-prefix="| | | " --cached -- file0 >result && > - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--line-prefix_--cached_--_file0" result > + process_diffs result >actual && > + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--line-prefix_--cached_--_file0" >expected && > + test_cmp expected actual > ' > > test_expect_success 'diff-tree --stdin with log formatting' ' >
On 2020-01-26 at 22:08:18, Johannes Schindelin wrote: > > diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > > index 5ac94b390d..6f5f05c3a8 100755 > > --- a/t/t4013-diff-various.sh > > +++ b/t/t4013-diff-various.sh > > @@ -120,6 +120,30 @@ test_expect_success setup ' > > +*++ [initial] Initial > > EOF > > > > +process_diffs () { > > + x04="[0-9a-f][0-9a-f][0-9a-f][0-9a-f]" && > > + x07="$_x05[0-9a-f][0-9a-f]" && > > Any reason not to stay with the convention, i.e. using `_x04` and `_x07` > here (with leading underscores)? None in particular. I have a slight bias against initial underscores from C, where that has a specific meaning, but I agree consistency is good, so I'll make that change. > > + sed -e "s/$OID_REGEX/$ZERO_OID/g" \ > > + -e "s/From $_x40 /From $ZERO_OID /" \ > > + -e "s/from $_x40)/from $ZERO_OID)/" \ > > + -e "s/commit $_x40\$/commit $ZERO_OID/" \ > > + -e "s/commit $_x40 (/commit $ZERO_OID (/" \ > > + -e "s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/" \ > > + -e "s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /" \ > > + -e "s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/" \ > > + -e "s/^$_x40 /$ZERO_OID /" \ > > + -e "s/^$_x40$/$ZERO_OID/" \ > > + -e "s/$x07\.\.$x07/fffffff..fffffff/g" \ > > + -e "s/$x07,$x07\.\.$x07/fffffff,fffffff..fffffff/g" \ > > + -e "s/$x07 $x07 $x07/fffffff fffffff fffffff/g" \ > > + -e "s/$x07 $x07 /fffffff fffffff /g" \ > > + -e "s/Merge: $x07 $x07/Merge: fffffff fffffff/g" \ > > + -e "s/$x07\.\.\./fffffff.../g" \ > > + -e "s/ $x04\.\.\./ ffff.../g" \ > > + -e "s/ $x04/ ffff/g" \ > > + "$1" > > +} > > + > > V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g') > > while read magic cmd > > do > > @@ -158,13 +182,15 @@ do > > } >"$actual" && > > if test -f "$expect" > > then > > + process_diffs "$actual" >actual && > > + process_diffs "$expect" >expect && > > case $cmd in > > *format-patch* | *-stat*) > > - test_i18ncmp "$expect" "$actual";; > > + test_i18ncmp expect actual;; > > *) > > - test_cmp "$expect" "$actual";; > > + test_cmp expect actual;; > > esac && > > - rm -f "$actual" > > + rm -f "$actual" actual expect > > else > > # this is to help developing new tests. > > cp "$actual" "$expect" > > @@ -383,16 +409,22 @@ test_expect_success 'log -S requires an argument' ' > > test_expect_success 'diff --cached on unborn branch' ' > > echo ref: refs/heads/unborn >.git/HEAD && > > git diff --cached >result && > > - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached" result > > + process_diffs result >actual && > > + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached" >expected && > > I was about to suggest letting `process_diffs` work in-place, but this > line makes that idea moot. If I could have done that, I would. > Another idea I had was to implement a `test_cmp_diff` that processes the > diffs and then compares them, but I guess that would be _less_ concise > than this patch. Yeah, this is a tricky test to work with because it does so many different things and trying to handle all of them in a tidy way is hard (as one can intuit from the giant sed statement). As part of the patch you quoted above, we sometimes use test_i18ncmp and sometimes use test_cmp here, so it's not easy hard to pick something that works for all of these cases without some duplication.
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 5ac94b390d..6f5f05c3a8 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -120,6 +120,30 @@ test_expect_success setup ' +*++ [initial] Initial EOF +process_diffs () { + x04="[0-9a-f][0-9a-f][0-9a-f][0-9a-f]" && + x07="$_x05[0-9a-f][0-9a-f]" && + sed -e "s/$OID_REGEX/$ZERO_OID/g" \ + -e "s/From $_x40 /From $ZERO_OID /" \ + -e "s/from $_x40)/from $ZERO_OID)/" \ + -e "s/commit $_x40\$/commit $ZERO_OID/" \ + -e "s/commit $_x40 (/commit $ZERO_OID (/" \ + -e "s/$_x40 $_x40 $_x40/$ZERO_OID $ZERO_OID $ZERO_OID/" \ + -e "s/$_x40 $_x40 /$ZERO_OID $ZERO_OID /" \ + -e "s/^$_x40 $_x40$/$ZERO_OID $ZERO_OID/" \ + -e "s/^$_x40 /$ZERO_OID /" \ + -e "s/^$_x40$/$ZERO_OID/" \ + -e "s/$x07\.\.$x07/fffffff..fffffff/g" \ + -e "s/$x07,$x07\.\.$x07/fffffff,fffffff..fffffff/g" \ + -e "s/$x07 $x07 $x07/fffffff fffffff fffffff/g" \ + -e "s/$x07 $x07 /fffffff fffffff /g" \ + -e "s/Merge: $x07 $x07/Merge: fffffff fffffff/g" \ + -e "s/$x07\.\.\./fffffff.../g" \ + -e "s/ $x04\.\.\./ ffff.../g" \ + -e "s/ $x04/ ffff/g" \ + "$1" +} + V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g') while read magic cmd do @@ -158,13 +182,15 @@ do } >"$actual" && if test -f "$expect" then + process_diffs "$actual" >actual && + process_diffs "$expect" >expect && case $cmd in *format-patch* | *-stat*) - test_i18ncmp "$expect" "$actual";; + test_i18ncmp expect actual;; *) - test_cmp "$expect" "$actual";; + test_cmp expect actual;; esac && - rm -f "$actual" + rm -f "$actual" actual expect else # this is to help developing new tests. cp "$actual" "$expect" @@ -383,16 +409,22 @@ test_expect_success 'log -S requires an argument' ' test_expect_success 'diff --cached on unborn branch' ' echo ref: refs/heads/unborn >.git/HEAD && git diff --cached >result && - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached" result + process_diffs result >actual && + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached" >expected && + test_cmp expected actual ' test_expect_success 'diff --cached -- file on unborn branch' ' git diff --cached -- file0 >result && - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result + process_diffs result >actual && + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" >expected && + test_cmp expected actual ' test_expect_success 'diff --line-prefix with spaces' ' git diff --line-prefix="| | | " --cached -- file0 >result && - test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--line-prefix_--cached_--_file0" result + process_diffs result >actual && + process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--line-prefix_--cached_--_file0" >expected && + test_cmp expected actual ' test_expect_success 'diff-tree --stdin with log formatting' '
This test produces a large number of diff formats and compares the output with test files that have content specific to SHA-1. Since we are more interested in the format of the diffs, and not their specific values, which are tested elsewhere, add a function which uses sed to transform these specific object IDs into generic ones of the right size, which we can then compare. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- t/t4013-diff-various.sh | 44 +++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-)