Re: [PATCH v6] format-patch --signature-file <file>

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v6] format-patch --signature-file <file>

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:17

Jeremiah Mahler [off-list ref] writes:
On Wed, May 21, 2014 at 02:13:06PM -0700, Junio C Hamano wrote:
quoted
Jeremiah Mahler [off-list ref] writes:
...
quoted
quoted
 	! grep "^-- \$" output
...
quoted
We have been trying not to do the above in recent test updates.  It
would be nice if this set-up did not have to be outside of the usual
test_expect_success structure.
Jeff caught those "! grep" instances in my patch.
Hmm, I didn't mean that one, and I do not offhand what is wrong
about "! grep" that says "output should not contain this string".

The problem is a "cat" you added outside test_expect_*; the recent
push is to have as little executable outside them, especially the
"set-up" code to prepare for the real tests.

i.e. we have been trying to write new tests (and convert old ones)
like this:

        test_expect_success 'I test such and such ' '
                cat >input-for-test <<-\EOF &&
                here comes input
                EOF
                git command-to-be-tested <input-for-test >actual &&
                cat >expected <<-\EOF &&
                here comes expected output
                EOF
                test_cmp expected actual
        '

not like this:

        cat >input-for-test <<-\EOF &&
        here comes input
        EOF
        test_expect_success 'I test such and such ' '
                git command-to-be-tested <input-for-test >actual &&
                cat >expected <<-\EOF &&
                here comes expected output
                EOF
                test_cmp expected actual
        '

Re: [PATCH v6] format-patch --signature-file <file>

From: Jeff King <hidden>
Date: 2016-06-15 23:01:17

On Wed, May 21, 2014 at 02:58:42PM -0700, Junio C Hamano wrote:
quoted
quoted
quoted
 	! grep "^-- \$" output
...
quoted
We have been trying not to do the above in recent test updates.  It
would be nice if this set-up did not have to be outside of the usual
test_expect_success structure.
Jeff caught those "! grep" instances in my patch.
Hmm, I didn't mean that one, and I do not offhand what is wrong
about "! grep" that says "output should not contain this string".
I think he is responding to my earlier request to use test_must_fail
instead of "!".  But there is a subtlety there he does not know, which
is that we typically only use the former for git programs, and rely on
"!" for normal Unix commands.
The problem is a "cat" you added outside test_expect_*; the recent
push is to have as little executable outside them, especially the
"set-up" code to prepare for the real tests.

i.e. we have been trying to write new tests (and convert old ones)
like this:

        test_expect_success 'I test such and such ' '
                cat >input-for-test <<-\EOF &&
                here comes input
                EOF
                git command-to-be-tested <input-for-test >actual &&
                cat >expected <<-\EOF &&
                here comes expected output
                EOF
                test_cmp expected actual
        '

not like this:

        cat >input-for-test <<-\EOF &&
        here comes input
        EOF
        test_expect_success 'I test such and such ' '
                git command-to-be-tested <input-for-test >actual &&
                cat >expected <<-\EOF &&
                here comes expected output
                EOF
                test_cmp expected actual
        '
Yeah, I noticed and gave a pass on this in earlier review, because the
file is used across many tests. So burying it in the first test that
uses it is probably a bad thing. However, it could go in its own setup
test.

-Peff

Re: [PATCH v6] format-patch --signature-file <file>

From: Jeremiah Mahler <hidden>
Date: 2016-06-15 23:01:17

On Wed, May 21, 2014 at 02:58:42PM -0700, Junio C Hamano wrote:
Jeremiah Mahler [off-list ref] writes:
...
The problem is a "cat" you added outside test_expect_*; the recent
push is to have as little executable outside them, especially the
"set-up" code to prepare for the real tests.

i.e. we have been trying to write new tests (and convert old ones)
like this:

        test_expect_success 'I test such and such ' '
                cat >input-for-test <<-\EOF &&
                here comes input
                EOF
                git command-to-be-tested <input-for-test >actual &&
                cat >expected <<-\EOF &&
                here comes expected output
                EOF
                test_cmp expected actual
        '

not like this:

        cat >input-for-test <<-\EOF &&
        here comes input
        EOF
        test_expect_success 'I test such and such ' '
                git command-to-be-tested <input-for-test >actual &&
                cat >expected <<-\EOF &&
                here comes expected output
                EOF
                test_cmp expected actual
        '
Now I understand.

Below is one of the updated test cases.

test_expect_success 'format-patch --signature-file=mail-signature' '
	cat >mail-signature <<-\EOF

	Test User [off-list ref]
	http://git.kernel.org/cgit/git/git.git

	git.kernel.org/?p=git/git.git;a=summary

	EOF
	git format-patch --stdout --signature-file=mail-signature -1 >output &&
	check_patch output &&
	sed -n -e "/^-- $/,\$p" <output | sed -e "1d" | sed -e "\$d" >output2 &&
	test_cmp mail-signature output2
'

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help