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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Eric,

On 2024-03-15 20:39, Eric Sunshine wrote:
On Fri, Mar 15, 2024 at 9:22 AM Dragan Simic <dsimic@xxxxxxxxxxx> wrote:
Add a handful of additional automated tests, to improve the coverage of configuration file entries whose values contain internal whitespace, leading and/or trailing whitespace, or which contain an additional inline comment.

Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>

Just some minor style-related comments...

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
@@ -11,6 +11,96 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+cat > .git/config << EOF
+[section]
+       solid = rock
+       sparse = big             blue
+       sparseAndTail = big              blue
+       sparseAndTailQuoted = "big               blue "
+       sparseAndBiggerTail = big                blue
+ sparseAndBiggerTailQuoted = "big blue " + sparseAndBiggerTailQuotedPlus = "big blue "
+       headAndTail =   big blue
+       headAndTailQuoted = "   big blue "
+       headAndTailQuotedPlus =  "      big blue "
+       annotated = big blue    # to be discarded
+       annotatedQuoted = "big blue"    # to be discarded
+EOF

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.

Use \EOF rather than EOF to signal to readers that we don't expect any
variable interpolation to happen within the here-doc body.

Further, use -\EOF inside test_expect_success() to allow us to indent
the body of the heredoc to match the test indentation.

Style guideline says to omit whitespace after redirection operators
(such as `<<` and `>`).

We have a q_to_tab() function which lets us state explicitly for
readers the location of TAB characters in the heredoc body. You'll
often see that used instead of literal TABs.

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.

Thank you for your review and all suggestions!  I'll make sure to rework
the tests in v2, in the way you described it above.

I'll come back with any questions I might have while reworking the new
tests.  I hope that's fine.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux