Re: [PATCH v3 3/3] patch-id-test: test --stable and --unstable flags
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 31, 2014, 19:29 UTC
- Message-ID
- <xmqqioqu5fr3.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1396202583-2572-3-git-send-email-mst@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
Show 26 quoted lines
> Verify that patch ID is now stable against diff split and reordering. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > Changes from v2: > added test to verify patch ID is stable against diff splitting > > t/t4204-patch-id.sh | 117 ++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 109 insertions(+), 8 deletions(-) > > diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh > index d2c930d..1679714 100755 > --- a/t/t4204-patch-id.sh > +++ b/t/t4204-patch-id.sh > @@ -5,12 +5,46 @@ test_description='git patch-id' > . ./test-lib.sh > > test_expect_success 'setup' ' > - test_commit initial foo a && > - test_commit first foo b && > - git checkout -b same HEAD^ && > - test_commit same-msg foo b && > - git checkout -b notsame HEAD^ && > - test_commit notsame-msg foo c > + cat > a <<-\EOF &&
Please do not add an extra space between the redirection operator (e.g. ">", "2>", etc.) and its operand. The same style violations everywhere in this patch.
Show 9 quoted lines
> + a > + a > + a > + a > + a > + a > + a > + a > + EOF
Please align EOF with the cat that receives it, and pretend that the column the head of the cat is at is the leftmost column for the payload, when writing <<- here-document, e.g.
cat >a <<-\EOF &&
a
EOF> + (cat a; echo b) > ab && > + (echo d; cat a; echo b) > dab && > + cp a foo && > + cp a bar &&
Probably a helper function would make it more apparent what is going on?
as="a a a a a a a a" && # eight a
test_write_lines $as b >ab &&
test_write_lines d $as b >dab &&
test_write_lines $as >foo &&
test_write_lines $as >bar &&Or even better, use test_write_lines in places you do use the result to overwrite foo/bar and get rid of ab and dab?
That helper can also be used to prepare bar-then-foo.
Show 8 quoted lines
> test_expect_success 'patch-id output is well-formed' '
> @@ -23,11 +57,33 @@ calc_patch_id () {
> sed "s# .*##" > patch-id_"$1"
> }
>
> +calc_patch_id_unstable () {
> + git patch-id --unstable |
> + sed "s# .*##" > patch-id_"$1"Not a new problem, but can you align "git patch-id" and "sed" to the same column? Also, not using "/" when there is no slash involved in the pattern makes readers waste their time wondering why the script avoids it.
Show 8 quoted lines
> +}
> +
> +calc_patch_id_stable () {
> + git patch-id --stable |
> + sed "s# .*##" > patch-id_"$1"
> +}
> +
> +Extra blank line.
Show 9 quoted lines
> get_patch_id () {
> - git log -p -1 "$1" | git patch-id |
> + git log -p -1 "$1" -O bar-then-foo -- | git patch-id |
> sed "s# .*##" > patch-id_"$1"
> }
>
> +get_patch_id_stable () {
> + git log -p -1 "$1" -O bar-then-foo | git patch-id --stable |
> + sed "s# .*##" > patch-id_"$1"Why doesn't it use calc_patch_id_stable?
Show 5 quoted lines
> +}
> +
> +get_patch_id_unstable () {
> + git log -p -1 "$1" -O bar-then-foo | git patch-id --unstable |
> + sed "s# .*##" > patch-id_"$1"Ditto.
> +} > + > +
Extra blank line.
Show 10 quoted lines
> test_expect_success 'patch-id detects equality' ' > get_patch_id master && > get_patch_id same && > @@ -52,10 +108,55 @@ test_expect_success 'patch-id supports git-format-patch output' ' > test_expect_success 'whitespace is irrelevant in footer' ' > get_patch_id master && > git checkout same && > - git format-patch -1 --stdout | sed "s/ \$//" | calc_patch_id same && > + git format-patch -1 --stdout | sed "s/ \$//" | > + calc_patch_id same &&
What is this change about?