| Message ID | cover-00.11-00000000000-20210423T072006Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | test-lib.sh: new test_commit args, simplification & fixes | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Changes since v4: Only a commit message change/re-wording per > <87v98e1oj7.fsf@evledraar.gmail.com>. > > Æ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() I wasn't paying much attention to this series, and didn't look at the last step at all (as I saw somebody else is already deeply biting into it), but the earlier parts looked good to me. It was painful to come back to the pile of topics after a week's interruption, though. While juggling ~50 topics, it is unfair to expect the maintainer to remember that a topic that is still going through iterations are depended on two other topics in flight (it is easier for contributors, who will be juggling far fewer number of topics at one time). But I think I've sorted out the duplicates caused by forgetting that I had to rebase ab/pickaxe-pcre2 and ab/describe-tests-fix on top of this topic. Will be pushing out the result of today's integration sometime this evening, which is far from complete as I am still in the "catch up with what happened during my absense" mode. Thanks.
On Fri, Apr 23 2021, Ævar Arnfjörð Bjarmason wrote: > Changes since v4: Only a commit message change/re-wording per > <87v98e1oj7.fsf@evledraar.gmail.com>. 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/xmqqsg30zdql.fsf@gitster.g/ > Æ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
On 2021-05-06 17:32:01+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > On Fri, Apr 23 2021, Ævar Arnfjörð Bjarmason wrote: > > > Changes since v4: Only a commit message change/re-wording per > > <87v98e1oj7.fsf@evledraar.gmail.com>. > > 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! I have some minor nit-picks here and there, I didn't look into 11/11. Otherwise, I think the series is fine, now.
Changes since v4: Only a commit message change/re-wording per <87v98e1oj7.fsf@evledraar.gmail.com>. Æ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