Re: [PATCH v3 1/7] t7063: correct broken test expectation

From: Derrick Stolee <stolee@gmail.com>
To: Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: "Martin Melka" <martin.melka@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Samuel Lijin" <sxlijin@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Elijah Newren" <newren@gmail.com>
Subject: Re: [PATCH v3 1/7] t7063: correct broken test expectation
Date: Thu, 26 Mar 2020 09:02:22 -0400	[thread overview]
Message-ID: <7ba7311c-08b8-8383-0281-79e58798e0be@gmail.com> (raw)
In-Reply-To: <d4fe5d335771e89dad40f717bf4623854d1efa9e.1585164718.git.gitgitgadget@gmail.com>

On 3/25/2020 3:31 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> The untracked cache is caching wrong information, resulting in commands
> like `git status --porcelain` producing erroneous answers.  The tests in
> t7063 actually have a wide enough test to catch a relevant case, in
> particular surrounding the directory 'dthree/', but it appears the
> answers were not checked quite closely enough and the tests were coded
> with the wrong expectation.  Once the wrong info got into the cache in
> an early test, since later tests built on it, many others have a wrong
> expectation as well.  This affects just over a third of the tests in
> t7063.

Wow. Good find.

> The error can be seen starting at t7063.12 (the first one switched from
> expect_success to expect_failure in this patch).  That test runs in a
> directory with the following files present:
>   done/one
>   dthree/three
>   dtwo/two
>   four
>   .gitignore
>   one
>   three
>   two
> 
> Of those files, the following files are tracked:
>   done/one
>   one
>   two
> 
> and the contents of .gitignore are:
>   four
> 
> and the contents of .git/info/exclude are:
>   three
> 
> And there is no core.excludesfile.  Therefore, the following should be
> untracked:
>   .gitignore
>   dthree/
>   dtwo/
> Indeed, these three paths are reported if you run
>   git ls-files -o --directory --exclude-standard
> within this directory.  However, 'git status --porcelain' was reporting
> for this test:
>   A  done/one
>   A  one
>   A  two
>   ?? .gitignore
>   ?? dtwo/
> which was clearly wrong -- dthree/ should also be listed as untracked.
> This appears to have been broken since the test was introduced with
> commit a3ddcefd97 ("t7063: tests for untracked cache", 2015-03-08).
> Correct the test to expect the right output, marking the test as failed
> for now.  Make the same change throughout the remainder of the testsuite
> to reflect that dthree/ remains an untracked directory throughout and
> should be recognized as such.

I wonder if we could simultaneously verify these "expected" results match
using another command without the untracked cache? It's good that we have
the expected outputs explicitly, but perhaps double-checking the command
with `-c core.untrackedCache=false` would help us know these are the correct
expected outputs?

Thanks,
-Stolee