Re: [PATCH v2 6/6] t2019: don't create unused files

Disclaimer: while trying to write a response to this email, I went a bit
off track, and a big portion of message below is an investigation of
test coverage of what is printed to standard output and standard error
by "git checkout".  It's still mostly relevant to the discussion about
t2019, or at least I hope so.

There are four parts to this investigation, starting with:

1. Standard output of "git checkout"
Other than "git checkout -p" (tested in t3701-add-interactive.sh), it
seems that the only thing that "git checkout" prints to standard output
is in:

   a) function "show_local_changes" in builtin/checkout.c -- a couple
      of tests in t7201-co.sh validate this
   b) function "report_tracking" in builtin/checkout.c -- there are
      tests that validate tracking information in output of "git
      checkout" in t6040-tracking-info.sh.  One test in
      t2020-checkout-detach.sh, titled 'tracking count is accurate after
      orphan check' validates it as well.

While trying to find all tests that validate standard output of "git checkout", I found out a couple of things.

2. Standard error of "git checkout"
Honestly, I haven't noticed it before, but I found it surprising that
messages about branch switching:

    - "Switched to branch '%s'\n"
    - "Switched to a new branch '%s'\n"
    - "Switched to and reset branch '%s'\n"

are printed to standard error.

Several tests in t2020-checkout-detach.sh validate what is printed into standard error by "git checkout" via a variation of ">actual 2>&1". Tests for advice printed by "git checkout" (looking at "advice.c", it all goes to stderr), also do a variation of ">actual 2>&1".

3. t2024-checkout-dwim.sh
Test 'loosely defined local base branch is reported correctly' in t2024
has an interesting validation of output of "git checkout":

	git checkout strict | sed -e "s/strict/BRANCHNAME/g" >expect &&
	status_uno_is_clean &&
	git checkout loose | sed -e "s/loose/BRANCHNAME/g" >actual &&
	status_uno_is_clean &&

	test_cmp expect actual

which is fine, except that neither file "expect" nor "actual" contain the string "BRANCHNAME". And this test was broken when it was introduced in 05e73682cd (checkout: report upstream correctly even with loosely defined branch.*.merge, 2014-10-14). It was probably intended for this test to redirect standard error of "git checkout". It should be cleaned up as a separate patch/topic.

On 06/04/2023 10:44, Ævar Arnfjörð Bjarmason wrote:

4. t2019-checkout-ambiguous-ref.sh
Back on topic: is empty standard output something that this test in
t2019 should worry about?  Let's take a look at other tests.

Aside from what was mentioned in section 1, tests in t7201 don't look at standard output of "git checkout". There isn't a lot of "test_must_be_empty" in t/t2*check*:

     $ git grep 'test_must_be_empty' t/t2*check*
     t/t2004-checkout-cache-temp.sh: test_must_be_empty stderr &&
     t/t2013-checkout-submodule.sh:  test_must_be_empty actual
     t/t2013-checkout-submodule.sh:  test_must_be_empty actual
     t/t2013-checkout-submodule.sh:  test_must_be_empty actual
     t/t2024-checkout-dwim.sh:       test_must_be_empty status.actual

The first one, in t2004, asserts output of "git checkout-index". All three in t2013 assert output of "git checkout HEAD >actual 2>&1". The last one, in t2024, asserts output of "git status".

(There's also one "test_line_count = 0" in the same test in t2004,
  but otherwise these tests seem to be pretty up-to-date w.r.t.
  to using test_must_be_empty helper)

My dumb script for finding unused files gives false-positives for such tests. And there a lot of tests that got split during introduction of C_LOCALE_OUTPUT prerequisite or were introduced before C_LOCALE_OUTPUT was phased out.

For t2019, however, the tests were created this way before C_LOCALE_OUTPUT in 0cb6ad3c3d ("checkout: fix bug with ambiguous refs", 2011-01-11). Then the prerequisite was added in 6b3d83efac ("t2019-checkout-ambiguous-ref.sh: depend on C_LOCALE_OUTPUT", 2011-04-03) and removed in d3bd0425b2 ("i18n: use test_i18ngrep in lib-httpd and t2019", 2011-04-12).