Re: [PATCH 1/5] t1300: write test expectations in the test's body
From: Patrick Steinhardt <hidden>
Date: 2025-09-15 11:41:24
On Thu, Sep 11, 2025 at 06:50:29PM +0200, Kristoffer Haugsbakk wrote:
On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:quoted
There are a bunch of tests in t1300 where we write the test expectation handed over to `test_cmp ()` outside of the test body. This does not match our modern test style, and there isn't really a reason why this would need to happen outside of the test bodies. Convert those to instead do so as part of the test itself. Note that there are two exceptions that we don't convert. In both of these cases the expectation is reused across multiple tests, and thus a conversion where we'd move that into the first test that uses the expectation would be invalid. Those are simply left as-is for now.This is just a suggestion (which everything is): Note that there are two exceptions that we leave as-is for now since they are reused across tests.
Yup, that reads way better.
quoted
index f856821839..bde9bda234 100755--- a/t/t1300-config.sh +++ b/t/t1300-config.sh@@ -134,38 +134,39 @@ test_expect_success 'clear default config' ' rm -f .git/config ' -cat > expect << EOF > +test_expect_success 'initial' ' + cat >expect <<EOF && [section] penguin = little blue EOF -test_expect_success 'initial' 'Ok. Correct. But I see that you are also overall doing a sort of normalization. • Remove-tabs if it makes sense • No variable expansion if it makes sense And here `<<\EOF` would work too. Or is that worse style? I will keep mentioning this throughout the rest.
No, it's not. I already did it sometimes, but not consistently. I'll mention this in the commit message and adapt as possible.
quoted
@@ -214,7 +213,9 @@ test_expect_success 'Prohibited LF in comment' ' test_must_fail git config ${mode_set} --comment="a${LF}b" section.k v ' -test_expect_success 'non-match result' 'test_cmp expect .git/config' +test_expect_success 'non-match result' ' + test_cmp expect .git/config +'Okay. You normalize the one line to test_expect_success <name> ' <body> ' The same kind of change done in the next patch `t1300: small style fixups`.
True, will move into the next patch. Thanks for going through all of these! Patrick