Subject: Re: [PATCH 1/2] t5318: use 'test_must_be_empty'

From: SZEDER Gábor <[email-masked]>
Date: Tue, 2 Jun 2020 20:04:03 +0200

On Mon, Jun 01, 2020 at 12:01:27PM -0600, Taylor Blau wrote:
> A handful of tests in t5318 use 'test_line_count = 0 ...' to make sure
> that some command does not write any output. While correct, it is more
> helpful to use 'test_must_be_empty' instead, since the latter prints the
> contents of the file if it is non-empty.
> 
> Since 'test_line_count' only prints the expected and actual line count,
> not the contents, using 'test_must_be_empty' may be more helpful for
> debugging if there is regression in any of these tests.

These two paragraphs essentially say the same thing, so I think only
one would be sufficient, but...  Both paragraphs are wrong, because
'test_line_count' does include the content of the file on failure:

  expecting success of 9999.1 'test': 
          cat >foo <<-EOF &&
          Add
          some
          content
          EOF
          test_line_count = 0 foo
  
  test_line_count: line count for foo != 0
  Add
  some
  content
  not ok 1 - test

Having said that, I think that the change itself is good, because
'test_must_be_empty foo' is more idiomatic.