Ævar Arnfjörð Bjarmason wrote:

Modernize test code added in ce567d1867a (Add test to show that
show-branch misses out the 8th column, 2008-07-23) and
11ee57bc4c4 (sort_in_topological_order(): avoid setting a commit flag,
2008-07-23) to use test helpers.

I'm renaming "out" to "actual" for consistency with other tests, and
introducing a "branches.sorted" file in the setup, to make it clear
that it's important that the list be sorted in this particular way.

That is better.

The "show-branch" output is indented with spaces, which would cause
complaints under "git show --check" with an indented here-doc
block. Let's prefix the lines with "> " to work around that, and to
make it clear that the leading whitespace is important.

I'm not sure this is an improvement. To me the original code is just
fine. Also, I don't think writing an 'expect' file belong in a setup
step.

Additionally I would do this change in a separate patch.

We can also get rid of the hardcoding of "main" added here in
334afbc76fb (tests: mark tests relying on the current default for
`init.defaultBranch`, 2020-11-18). For this test we're setting up an
"initial" commit anyway, and now that we've moved over to test_commit
we can reference that instead.

That's also good.

All the changes in this patch look good to me, however, they are smashed
together in a way that makes the review harder, I see:

 1. Use test_commit
 2. Rename out to actual
 3. Use >actual instead of > actual
 4. Use test_seq instead of $numbers
 5. Use branches.sorted instead of branch$i
 6. Use test_config instead of git config
 7. Use internal sed 's/^ //' instead of outside cat

I'm on-board with 6 out of 7, but if these were done it at least 2
patches, they would be clearer. I in fact would prefer one patch per
change (although maybe squash 3 with 2).

Cheers.

-- 
Felipe Contreras