| Message ID | 20211208163335.1231795-9-fs@gigacodes.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ssh signing: verify key lifetime | expand |
Fabian Stelzer <fs@gigacodes.de> writes: > All the GPG, GPGSSH and the new GPGSSH_VERIFYTIME tests are redirecing > stdout as well as stderr to `actual` and grep for success/failure over > the resulting flie. However, no output is printed on stderr and we do > not need to include it in the grep. The newer SSH signing based tests > are also missing a grep for the merged tag to make sure the merge > message is correct. > > - remove unneccessary 2>&1 redirects > - add grep for merged tag to gpgssh* tests > > Signed-off-by: Fabian Stelzer <fs@gigacodes.de> > --- > t/t6200-fmt-merge-msg.sh | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) The 4 hunks at the beginning seems to be a clean-up of the existing issue, but aren't hunks -162,7, -170,7, -178,7, 187,7 "oops, we screwed up in [6/8], and we patch up after the fact"? For a new topic not yet in 'next', we'd prefer to pretend to be more perfect humans by not deliberatly keeping mistakes made in an earlier step, only to be corrected in a later step. While there may not be any difference in the end-result, it would be cleaner and less sloppy to have the 4 hunks to fix the ones before the series as a preliminary clean-up step, and the other hunks folded into the step that introduced the problem, no? Thanks. > diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh > index 2dd2423643..12a1e62bf0 100755 > --- a/t/t6200-fmt-merge-msg.sh > +++ b/t/t6200-fmt-merge-msg.sh > @@ -124,7 +124,7 @@ test_expect_success 'message for merging local branch' ' > test_expect_success GPG 'message for merging local tag signed by good key' ' > git checkout main && > git fetch . signed-good-tag && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > grep "^Merge tag ${apos}signed-good-tag${apos}" actual && > grep "^# gpg: Signature made" actual && > grep "^# gpg: Good signature from" actual > @@ -133,7 +133,7 @@ test_expect_success GPG 'message for merging local tag signed by good key' ' > test_expect_success GPG 'message for merging local tag signed by unknown key' ' > git checkout main && > git fetch . signed-good-tag && > - GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual && > grep "^Merge tag ${apos}signed-good-tag${apos}" actual && > grep "^# gpg: Signature made" actual && > grep -E "^# gpg: Can${apos}t check signature: (public key not found|No public key)" actual > @@ -143,7 +143,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by good ssh key > test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && > git checkout main && > git fetch . signed-good-ssh-tag && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && > ! grep "${GPGSSH_BAD_SIGNATURE}" actual > ' > @@ -152,7 +152,8 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh > test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && > git checkout main && > git fetch . signed-untrusted-ssh-tag && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > + grep "^Merge tag ${apos}signed-untrusted-ssh-tag${apos}" actual && > grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual && > ! grep "${GPGSSH_BAD_SIGNATURE}" actual && > grep "${GPGSSH_KEY_NOT_TRUSTED}" actual > @@ -162,7 +163,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign > test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && > git checkout main && > git fetch . expired-signed && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > + grep "^Merge tag ${apos}expired-signed${apos}" actual && > ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual > ' > > @@ -170,7 +172,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign > test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && > git checkout main && > git fetch . notyetvalid-signed && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > + grep "^Merge tag ${apos}notyetvalid-signed${apos}" actual && > ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual > ' > > @@ -178,7 +181,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign > test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && > git checkout main && > git fetch . timeboxedvalid-signed && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > + grep "^Merge tag ${apos}timeboxedvalid-signed${apos}" actual && > grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && > ! grep "${GPGSSH_BAD_SIGNATURE}" actual > ' > @@ -187,7 +191,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign > test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && > git checkout main && > git fetch . timeboxedinvalid-signed && > - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && > + git fmt-merge-msg <.git/FETCH_HEAD >actual && > + grep "^Merge tag ${apos}timeboxedinvalid-signed${apos}" actual && > ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual > '
On 08.12.2021 15:20, Junio C Hamano wrote: >Fabian Stelzer <fs@gigacodes.de> writes: > >> All the GPG, GPGSSH and the new GPGSSH_VERIFYTIME tests are redirecing >> stdout as well as stderr to `actual` and grep for success/failure over >> the resulting flie. However, no output is printed on stderr and we do >> not need to include it in the grep. The newer SSH signing based tests >> are also missing a grep for the merged tag to make sure the merge >> message is correct. >> >> - remove unneccessary 2>&1 redirects >> - add grep for merged tag to gpgssh* tests >> >> Signed-off-by: Fabian Stelzer <fs@gigacodes.de> >> --- >> t/t6200-fmt-merge-msg.sh | 21 +++++++++++++-------- >> 1 file changed, 13 insertions(+), 8 deletions(-) > >The 4 hunks at the beginning seems to be a clean-up of the existing >issue, but aren't hunks -162,7, -170,7, -178,7, 187,7 "oops, we >screwed up in [6/8], and we patch up after the fact"? > >For a new topic not yet in 'next', we'd prefer to pretend to be more >perfect humans by not deliberatly keeping mistakes made in an >earlier step, only to be corrected in a later step. > >While there may not be any difference in the end-result, it would be >cleaner and less sloppy to have the 4 hunks to fix the ones before >the series as a preliminary clean-up step, and the other hunks >folded into the step that introduced the problem, no? > Ok, i will send a new version with the fixes for the earlier tests in extra commits. Your 5.1 includes a +"grep "^Merge tag ${apos}signed-untrusted-ssh-tag${apos}" actual &&" fix for a preexisting test in the first patch which now claims to only remove the stderr redirects though. Also another grep for the merged tag for an earlier test got lost somewhere. I'll send a 5.2 Thanks
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 2dd2423643..12a1e62bf0 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -124,7 +124,7 @@ test_expect_success 'message for merging local branch' ' test_expect_success GPG 'message for merging local tag signed by good key' ' git checkout main && git fetch . signed-good-tag && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-tag${apos}" actual && grep "^# gpg: Signature made" actual && grep "^# gpg: Good signature from" actual @@ -133,7 +133,7 @@ test_expect_success GPG 'message for merging local tag signed by good key' ' test_expect_success GPG 'message for merging local tag signed by unknown key' ' git checkout main && git fetch . signed-good-tag && - GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "^Merge tag ${apos}signed-good-tag${apos}" actual && grep "^# gpg: Signature made" actual && grep -E "^# gpg: Can${apos}t check signature: (public key not found|No public key)" actual @@ -143,7 +143,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by good ssh key test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && git checkout main && git fetch . signed-good-ssh-tag && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual ' @@ -152,7 +152,8 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && git checkout main && git fetch . signed-untrusted-ssh-tag && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && + grep "^Merge tag ${apos}signed-untrusted-ssh-tag${apos}" actual && grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual && grep "${GPGSSH_KEY_NOT_TRUSTED}" actual @@ -162,7 +163,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && git checkout main && git fetch . expired-signed && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && + grep "^Merge tag ${apos}expired-signed${apos}" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' @@ -170,7 +172,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && git checkout main && git fetch . notyetvalid-signed && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && + grep "^Merge tag ${apos}notyetvalid-signed${apos}" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual ' @@ -178,7 +181,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && git checkout main && git fetch . timeboxedvalid-signed && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && + grep "^Merge tag ${apos}timeboxedvalid-signed${apos}" actual && grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual && ! grep "${GPGSSH_BAD_SIGNATURE}" actual ' @@ -187,7 +191,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && git checkout main && git fetch . timeboxedinvalid-signed && - git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 && + git fmt-merge-msg <.git/FETCH_HEAD >actual && + grep "^Merge tag ${apos}timeboxedinvalid-signed${apos}" actual && ! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual '
All the GPG, GPGSSH and the new GPGSSH_VERIFYTIME tests are redirecing stdout as well as stderr to `actual` and grep for success/failure over the resulting flie. However, no output is printed on stderr and we do not need to include it in the grep. The newer SSH signing based tests are also missing a grep for the merged tag to make sure the merge message is correct. - remove unneccessary 2>&1 redirects - add grep for merged tag to gpgssh* tests Signed-off-by: Fabian Stelzer <fs@gigacodes.de> --- t/t6200-fmt-merge-msg.sh | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)