Re: [PATCH 3/4] t1300: add more tests for whitespace and inline comments

On Fri, Mar 15, 2024 at 3:39 PM Eric Sunshine [off-list ref] wrote:

These days we try to place all test-related code inside a
test_expect_success() context rather than having it standalone. In
this case, since the file being created is (presumably) shared by
multiple tests in this script, you may want to add a new test which
performs this setup step.

Taking all the above into account, perhaps:

    test_expect_success 'setup whitespace' '
        q_to_tab >.git/config <<-\EOF
        [section]
        solid = rock
        sparse = bigQblue
        ...
        EOF

Same comments apply to rest of patch.

To be clear, this case is special because the file being created is
shared by multiple tests, so it deserves being placed in its own
test_expect_success() invocation.

For the remaining cases where you're doing some set-up outside of
test_expect_success(), just move the set-up code into the
corresponding test_expect_success() invocation. For instance, rather
than:

    echo 'big               blue' > expect

    test_expect_success 'internal whitespace' '
        git config --get section.sparse > output &&
        test_cmp expect output
    '

do this:

    test_expect_success 'internal whitespace' '
        echo 'bigQblue' | q_to_tab >expect
        git config --get section.sparse >actual &&
        test_cmp expect actual
    '

(I changed "output" to "actual" above since the names "expect" and
"actual" are common in the tests.)