Re: [PATCH 3/4] t1300: add more tests for whitespace and inline comments
- From
- Dragan Simic <dsimic@manjaro.org>
- Date
- Mar 15, 2024, 21:42 UTC
- Message-ID
- <ac99ea0a24c257eab8f29466589b6eef@manjaro.org>
- In-Reply-To
- <CAPig+cRG8eFxepkaiN54H+fa7D=rFGsmEHdvTP+HSSaLO_6T_A@mail.gmail.com>
On 2024-03-15 21:29, Eric Sunshine wrote:
Show 46 quoted lines
> On Fri, Mar 15, 2024 at 3:39 PM Eric Sunshine <sunshine@sunshineco.com> > 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.)
This looks nice, thanks again. It keeps the expected results and the test execution in a single "block", making it a bit easier to keep track of different tests and their expected results.