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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jun 2, 2020, 18:04 UTC
Message-ID: <20200602180403.GA4791@szeder.dev>
In-Reply-To: <fddbe588147444fe2675224c94f50e24a1d12871.1591034353.git.me@ttaylorr.com>

On Mon, Jun 01, 2020 at 12:01:27PM -0600, Taylor Blau wrote:

Show 8 quoted lines

> 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.