Junio C Hamano <gitster@xxxxxxxxx> writes: >> + git config fetch.parallel 0 && > > Is this necessary for the purpose of the test, though? It should > not hurt, but we do not require the end-users to set it in real life > for the parallel fetching to work, either, right? IIUC it would make the test output deterministic if we fetched from both remotes. That doesn't happen here though, so I guess it's not doing anything right now. >> + git fetch --all --no-recurse-submodules 2>../actual >> + ) && >> + >> + cat >expect <<-EOF && >> + From ../src >> + * [new branch] master -> secondary/master >> + EOF >> + test_cmp expect actual >> +' > > In the context of a series that attempts to introduce a new stable > output format for machine consumption, which implies the traditional > output can change to match human users' preference, this test feels > a bit brittle, but let's wait until the end of the series to judge > that. I also find it a bit brittle to assert on the whole output when this test is about checking that we do not fetch the superproject. Is there a reason you didn't go with the "grep for submodule lines" approach in the previous tests? If it's about catching regressions, IMO your PATCH 2/8 does a good enough job of doing that. Wondering out loud, I wonder if it makes sense for us to make a bigger distinction between "tests whose purpose is to guard against unexpected changes in output" (i.e. snapshot tests) vs "tests that happen to use output as a way to assert behavior" (i.e. 'regular' behavioral tests). Many GUI app codebases have such a distinction and have different best practices around them.
