On Wed, May 21, 2014 at 02:58:42PM -0700, Junio C Hamano wrote: > >> > ! grep "^-- \$" output > > ... > >> > >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html