Subject: Re: [PATCH v5 00/11] test-lib.sh: new test_commit args, simplification & fixes

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 06 May 2021 17:32:01 +0200

On Fri, Apr 23 2021, Ævar Arnfjörð Bjarmason wrote:

> Changes since v4: Only a commit message change/re-wording per
> <[email-masked]>.

SZEDER, Eric, Đoàn et al: does this v5 look good to you? Junio's
suggested in [1] that this needs some final review feedback before
advancing to 'next'. If you could take a look that would be great!

1. https://lore.kernel.org/git/[email-masked]/

> Ævar Arnfjörð Bjarmason (11):
>   test-lib: bring $remove_trash out of retirement
>   test-lib tests: remove dead GIT_TEST_FRAMEWORK_SELFTEST variable
>   test-lib-functions: reword "test_commit --append" docs
>   test-lib-functions: document test_commit --no-tag
>   test-lib functions: add an --annotated option to "test_commit"
>   describe tests: convert setup to use test_commit
>   test-lib functions: add --printf option to test_commit
>   submodule tests: use symbolic-ref --short to discover branch name
>   test-lib: reformat argument list in test_create_repo()
>   test-lib: do not show advice about init.defaultBranch under --verbose
>   test-lib: split up and deprecate test_create_repo()
>
>  t/lib-submodule-update.sh           |  3 +-
>  t/t0000-basic.sh                    |  4 --
>  t/t1307-config-blob.sh              |  4 +-
>  t/t1403-show-ref.sh                 |  6 +--
>  t/t2030-unresolve-info.sh           |  3 +-
>  t/t4006-diff-mode.sh                |  6 +--
>  t/t4030-diff-textconv.sh            |  8 +---
>  t/t5406-remote-rejects.sh           |  1 -
>  t/t5407-post-rewrite-hook.sh        |  2 -
>  t/t5409-colorize-remote-messages.sh |  1 -
>  t/t5520-pull.sh                     | 10 +----
>  t/t6120-describe.sh                 | 58 +++++++---------------------
>  t/test-lib-functions.sh             | 60 ++++++++++++++++++-----------
>  t/test-lib.sh                       | 40 +++++++++++--------
>  14 files changed, 87 insertions(+), 119 deletions(-)
>
> Range-diff against v4:
>  1:  a76ea749bb6 =  1:  75667f98f3a test-lib: bring $remove_trash out of retirement
>  2:  de7be7844ea =  2:  55c9413f9cd test-lib tests: remove dead GIT_TEST_FRAMEWORK_SELFTEST variable
>  3:  709bc773fb0 =  3:  361e34654e9 test-lib-functions: reword "test_commit --append" docs
>  4:  b67654334e6 =  4:  2db68a4ac7c test-lib-functions: document test_commit --no-tag
>  5:  3a166c92063 =  5:  4ceba3d404b test-lib functions: add an --annotated option to "test_commit"
>  6:  981fc43ee69 =  6:  589eaf7a078 describe tests: convert setup to use test_commit
>  7:  15057cdecfe =  7:  a0fe0640148 test-lib functions: add --printf option to test_commit
>  8:  5d437f53ec8 =  8:  7fb8849ce66 submodule tests: use symbolic-ref --short to discover branch name
>  9:  9ee13ee71bc =  9:  f67245ba40d test-lib: reformat argument list in test_create_repo()
> 10:  6ba568df9f4 = 10:  37338c88300 test-lib: do not show advice about init.defaultBranch under --verbose
> 11:  311a9dba36b ! 11:  7793311e5f1 test-lib: split up and deprecate test_create_repo()
>     @@ Commit message
>              from "mkdir .git/hooks" changes various tests needed to re-setup
>              that directory. Now they no longer do.
>      
>     -     5. Since we don't need to move the .git/hooks directory we don't need
>     -        the subshell here either.
>     -
>     -        See 0d314ce834 for when the subshell use got introduced for the
>     -        convenience of not having to "cd" back and forth while setting up
>     -        the hooks.
>     +        This makes us implicitly depend on the default hooks being
>     +        disabled, which is a good thing. If and when we'd have any
>     +        on-by-default hooks (I see no reason we ever would) we'd want to
>     +        see the subtle and not so subtle ways that would break the test
>     +        suite.
>     +
>     +     5. We don't need to "cd" to the "$repo" directory at all anymore.
>     +
>     +        In the code being removed here we both "cd"'d to the repository
>     +        before calling "init", and did so in a subshell.
>     +
>     +        It's not important to do either, so both of those can be
>     +        removed. We cd'd because this code grew from test-lib.sh code
>     +        where we'd have done so already, see eedf8f97e58 (Abstract
>     +        test_create_repo out for use in tests., 2006-02-17), and later
>     +        "cd"'d inside a subshell since 0d314ce834d to avoid having to keep
>     +        track of an "old pwd" variable to cd back after the setup.
>     +
>     +        Being in the repository directory made moving the hooks around
>     +        easier (we wouldn't have to fully qualify the path). Since we're
>     +        not moving the hooks per #4 above we don't need to "cd" for that
>     +        reason either.
>      
>           6. We can drop the --template argument and instead rely on the
>              GIT_TEMPLATE_DIR set to the same path earlier in test-lib.sh. See