Re: [PATCH v2 11/12] t5524: test --log=1 limits shortlog length

Johannes Schindelin [off-list ref] writes:

Hi Hannes,

On 2015-05-07 21:12, Johannes Sixt wrote:

quoted

Am 07.05.2015 um 19:06 schrieb Paul Tan:

quoted

On Fri, May 8, 2015 at 12:28 AM, Johannes Schindelin
[off-list ref] wrote:

quoted

On 2015-05-07 10:44, Paul Tan wrote:

quoted

@@ -32,4 +35,18 @@ test_expect_success pull '
  )
  '

+test_expect_failure '--log=1 limits shortlog length' '
+(
+     cd cloned &&
+     git reset --hard HEAD^ &&
+     test `cat afile` = original &&
+     test `cat bfile` = added &&
+     git pull --log &&
+     git log -3 &&
+     git cat-file commit HEAD >result &&
+     grep Dollar result &&
+     ! grep "second commit" result
+)

I think it might be better to use `test_must_fail` here, just for
consistency (the `!` operator would also pass if `grep` itself could not
be executed correctly, quite academic, I know, given that `grep` is
exercised plenty of times by the test suite, but still...)

What do you think?

Yep, it's definitely better. Sometimes I forget about the existence of
some test utility functions :-/.

Nope, it's not better. test_must_fail is explicitly only for git
invocations. We do not expect 'grep' to segfault or something.

Cf. eg.
http://thread.gmane.org/gmane.comp.version-control.git/258725/focus=258752

That link leads to a patch that changes `! grep` to a `test_must_fail grep` and is not contested, at least not in the thread visible on GMane. Would you have a link with a more convincing argument for me?

t/README states:

   On the other hand, don't use test_must_fail for running regular
   platform commands; just use '! cmd'.  We are not in the business
   of verifying that the world given to us sanely works.

Except for a few cases that is also respected in the test scripts.

$ git grep "! grep" | wc -l
203
$ git grep "test_must_fail grep" | wc -l
19

So I think using ! grep is the right way to go.

Thank you,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html