| Message ID | cover.1590019226.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | t: replace incorrect test_must_fail usage (part 5) | expand |
Denton Liu <liu.denton@gmail.com> writes: > Hi all, > > This is mostly a resend of what's currently queued in > "dl/test-must-fail-fixes-5" except with a tiny bit of cleanup on the tip > patch. I'd appreciate a review on this series so that we can finally get > rid of that "Needs review" on the What's Cooking messages ;) Thanks. The OVERWRITING_FAIL one was the only one I was unhappy about, so it would be good to have more eyeballs on it---perhaps other people find the approach acceptable, or can suggest more readable and understandable approach.
On Thu, May 21, 2020 at 09:47:22AM -0700, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > Hi all, > > > > This is mostly a resend of what's currently queued in > > "dl/test-must-fail-fixes-5" except with a tiny bit of cleanup on the tip > > patch. I'd appreciate a review on this series so that we can finally get > > rid of that "Needs review" on the What's Cooking messages ;) > > Thanks. > > The OVERWRITING_FAIL one was the only one I was unhappy about, so it > would be good to have more eyeballs on it---perhaps other people > find the approach acceptable, or can suggest more readable and > understandable approach. FWIW, I don't really like it either. :) I gave my best shot at an alternative in reply to that patch. -Peff
Hi all, This is mostly a resend of what's currently queued in "dl/test-must-fail-fixes-5" except with a tiny bit of cleanup on the tip patch. I'd appreciate a review on this series so that we can finally get rid of that "Needs review" on the What's Cooking messages ;) The overall scope of these patches is to replace inappropriate uses of test_must_fail. IOW, we should only allow test_must_fail to run on `git` and `test-tool`. Ultimately, we will conclude by making test_must_fail error out on non-git commands. An advance view of the final series can be found here[1]. This is the fifth part. It focuses on lib-submodule-update.sh and tests that make use of it. The first part can be found here[2]. The second part can be found here[3]. The third part can be found here[4]. The fourth part can be found here[5]. Changes since v1.2: * In "lib-submodule-update: pass OVERWRITING_FAIL", use if-then return to reduce the amount of code churn [1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2 [2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/ [3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/ [4]: https://lore.kernel.org/git/cover.1585209554.git.liu.denton@gmail.com/ [5]: https://lore.kernel.org/git/cover.1587372771.git.liu.denton@gmail.com/ Denton Liu (4): lib-submodule-update: add space after function name lib-submodule-update: consolidate --recurse-submodules lib-submodule-update: prepend "git" to $command lib-submodule-update: pass OVERWRITING_FAIL t/lib-submodule-update.sh | 55 ++++++++++++++++++++++---------- t/t1013-read-tree-submodule.sh | 4 +-- t/t2013-checkout-submodule.sh | 4 +-- t/t3426-rebase-submodule.sh | 8 ++--- t/t3512-cherry-pick-submodule.sh | 2 +- t/t3513-revert-submodule.sh | 7 +++- t/t3906-stash-submodule.sh | 8 +++-- t/t4137-apply-submodule.sh | 10 +++--- t/t4255-am-submodule.sh | 10 +++--- t/t5572-pull-submodule.sh | 16 +++++----- t/t6041-bisect-submodule.sh | 8 +++-- t/t7112-reset-submodule.sh | 6 ++-- t/t7613-merge-submodule.sh | 8 ++--- 13 files changed, 92 insertions(+), 54 deletions(-) Range-diff against v1: 1: f148526dae = 1: ba2f642e0f lib-submodule-update: add space after function name 2: b7d7d0d761 = 2: 16d0a3eb9a lib-submodule-update: consolidate --recurse-submodules 3: 4922e75f2a = 3: 578bab6f1a lib-submodule-update: prepend "git" to $command 4: a0a8fbd881 ! 4: 48598e3f98 lib-submodule-update: pass OVERWRITING_FAIL @@ Commit message with a file buffer so that the return code of the first command is not lost. - This patch can be better viewed with `--ignore-all-space`. - ## t/lib-submodule-update.sh ## @@ t/lib-submodule-update.sh: test_submodule_content () { # a removed submodule. @@ t/t3513-revert-submodule.sh: git_revert () { git status -su >expect && ls -1pR * >>expect && tar cf "$TRASH_DIRECTORY/tmp.tar" * && -- git checkout "$1" && -- git revert HEAD && -- rm -rf * && -- tar xf "$TRASH_DIRECTORY/tmp.tar" && -- git status -su >actual && -- ls -1pR * >>actual && -- test_cmp expect actual && -- git revert HEAD + $OVERWRITING_FAIL git checkout "$1" && -+ if test -z "$OVERWRITING_FAIL" ++ if test -n "$OVERWRITING_FAIL" + then -+ git checkout "$1" && -+ git revert HEAD && -+ rm -rf * && -+ tar xf "$TRASH_DIRECTORY/tmp.tar" && -+ git status -su >actual && -+ ls -1pR * >>actual && -+ test_cmp expect actual && -+ git revert HEAD -+ fi - } - - KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 ++ return ++ fi && + git checkout "$1" && + git revert HEAD && + rm -rf * && ## t/t3906-stash-submodule.sh ## @@ t/t3906-stash-submodule.sh: test_description='stash can handle submodules' @@ t/t3906-stash-submodule.sh: test_description='stash can handle submodules' git status -su >expect && ls -1pR * >>expect && - git read-tree -u -m "$1" && -- git stash && -- git status -su >actual && -- ls -1pR * >>actual && -- test_cmp expect actual && -- git stash apply + $OVERWRITING_FAIL git read-tree -u -m "$1" && -+ if test -z "$OVERWRITING_FAIL" ++ if test -n "$OVERWRITING_FAIL" + then -+ git stash && -+ git status -su >actual && -+ ls -1pR * >>actual && -+ test_cmp expect actual && -+ git stash apply -+ fi - } - - KNOWN_FAILURE_STASH_DOES_IGNORE_SUBMODULE_CHANGES=1 ++ return ++ fi && + git stash && + git status -su >actual && + ls -1pR * >>actual && ## t/t4137-apply-submodule.sh ## @@ t/t4137-apply-submodule.sh: test_description='git apply handling submodules' @@ t/t6041-bisect-submodule.sh: git_bisect () { tar cf "$TRASH_DIRECTORY/tmp.tar" * && GOOD=$(git rev-parse --verify HEAD) && - git checkout "$1" && -- echo "foo" >bar && -- git add bar && -- git commit -m "bisect bad" && -- BAD=$(git rev-parse --verify HEAD) && -- git reset --hard HEAD^^ && -- git submodule update && -- git bisect start && -- git bisect good $GOOD && -- rm -rf * && -- tar xf "$TRASH_DIRECTORY/tmp.tar" && -- git status -su >actual && -- ls -1pR * >>actual && -- test_cmp expect actual && -- git bisect bad $BAD + $OVERWRITING_FAIL git checkout "$1" && -+ if test -z "$OVERWRITING_FAIL" ++ if test -n "$OVERWRITING_FAIL" + then -+ echo "foo" >bar && -+ git add bar && -+ git commit -m "bisect bad" && -+ BAD=$(git rev-parse --verify HEAD) && -+ git reset --hard HEAD^^ && -+ git submodule update && -+ git bisect start && -+ git bisect good $GOOD && -+ rm -rf * && -+ tar xf "$TRASH_DIRECTORY/tmp.tar" && -+ git status -su >actual && -+ ls -1pR * >>actual && -+ test_cmp expect actual && -+ git bisect bad $BAD -+ fi - } - - test_submodule_switch_func "git_bisect" ++ return ++ fi && + echo "foo" >bar && + git add bar && + git commit -m "bisect bad" &&