On Mon, May 24, 2021 at 04:38:18PM -0400, Derrick Stolee wrote: > On 5/24/21 4:28 PM, Jonathan Nieder wrote: > > Hm, I think this test strategy is going to be fundamentally flaky > > regardless: Git doesn't intend to guarantee any kind of stability in > > the exact stderr output it writes. > > There are no expectations that stderr is stable across > versions of Git. These tests don't add friction to developers > making new features or changing the error messages that appear > over stderr. It's just that these tests should catch any > unintended inconsistency across these modes. I agree with Stolee that these tests are valuable for asserting that output is the consistent whether or not you are using the sparse index. I find setting GIT_PROGRESS_DELAY to a large number to a be a little ugly, but there isn't an apparent better way to accomplish the same thing. Of course, it would be nice to have an environment variable to specify where progress meters are written to, or a global option to disable progress meters altogether. But I don't think this isolated instance should push in the direction of adding support for either of the above, regardless of how easy it might be. What would perhaps make more sense is to silence the progress meters from the commands themselves. AFAICT the only command called by run_on_sparse() which generates a progress meter is 'git checkout', 'git merge', and 'git submodule', all of which support '--no-progress'. Might it be worth passing that option instead of setting GIT_PROGRESS_DELAY to a large value? (For what it's worth, I have no strong opinion either way, so I would be happy to attach my Reviewed-by to even the current version of this patch). Thanks, Taylor
