From patchwork Thu Apr 30 10:25:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Denton Liu X-Patchwork-Id: 11519771 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 26E14912 for ; Thu, 30 Apr 2020 10:25:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 046A82137B for ; Thu, 30 Apr 2020 10:25:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="c5Lsi3jx" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726413AbgD3KZ2 (ORCPT ); Thu, 30 Apr 2020 06:25:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50778 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1726127AbgD3KZ2 (ORCPT ); Thu, 30 Apr 2020 06:25:28 -0400 Received: from mail-qv1-xf43.google.com (mail-qv1-xf43.google.com [IPv6:2607:f8b0:4864:20::f43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DEC01C035494 for ; Thu, 30 Apr 2020 03:25:26 -0700 (PDT) Received: by mail-qv1-xf43.google.com with SMTP id y19so2698713qvv.4 for ; Thu, 30 Apr 2020 03:25:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=5A5MWVwn5UXNotysOtDS7LZzWQutmJZjV2HkTC7kKRs=; b=c5Lsi3jxEoiXRttVg9RJItrZJ/VF8l6SpmDBxUVm3QWJfTaVrsCdeKPzLAztLipHds T7wMVylSnqvO6lBQ7fxlVzY29P7+AAiFSbYm9CTqre4oyb3BWSyTm05ZH5G9AUMi9Baj /uTsWnlWiGCBo+HiNSlYyZEAYgXyFLJv/zsl2DRh6eK6Hrvn+K+poMF6R/Ssw7ngBjyn TAWXDcCBYHL5C2CyjFQ5Q9tF41xMmgt7B8J3r0d0c37VcpJRT4HuS71q/4cskc7UdvIM C4Rii9lAIdgdjGohH6nqj68WqPidHu5P4tregTngoXvATYsmBnUHv40HzudlcfVCym92 mFig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=5A5MWVwn5UXNotysOtDS7LZzWQutmJZjV2HkTC7kKRs=; b=Uv2WUWGS3aBaJ5JOkQ+jf/tU6542ksDf6kqX8HAhZb46FUarE85J0lBFXDfiKShgRn mbHlS9zN+PMdUnlAIZEP5pDpz+/iwhF8BaTr5TXe2xkH/55o61S2U1C3mn87UEZGTlYG 1vQF9h7q5rHWSD4YVE++GZG/w5YRYbrS5KwuB25zA5UTOimr2ebC8j+9ZDqa648irGTU 4VctZ8vEc9gvh9OM5bgsR0ll1lWZ0G6/DVbWMZYdh9bt9pXBjrYygjiLaW0d3jaN+qva LWf4wS1B6cwJ5QaaZyrXX07DY/ofn1ol7kaUUlRK9rBUgJz4gkOpwcK1eKIHs/4Nnp40 aIvg== X-Gm-Message-State: AGi0Pubea+6vPk4j+e+h/jPtEVePOCDKKboHRcVZaEstfdUTGWndsNsk KHDTf69N1d6n5Yl8dWdz5oDHE+b0 X-Google-Smtp-Source: APiQypJzj/x6+PKJNnzoRU8WNCeWidJqrBNH/rniSTQKFd94w0llalVwRv9ENcEu/+g1MOWpg8Bpog== X-Received: by 2002:a0c:eb09:: with SMTP id j9mr2269974qvp.196.1588242325685; Thu, 30 Apr 2020 03:25:25 -0700 (PDT) Received: from archbookpro.localdomain (CPE18593399858a-CM185933998587.cpe.net.cable.rogers.com. [174.112.65.113]) by smtp.gmail.com with ESMTPSA id j90sm1746871qte.20.2020.04.30.03.25.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2020 03:25:25 -0700 (PDT) From: Denton Liu To: Git Mailing List Cc: Junio C Hamano Subject: [PATCH v1.1] lib-submodule-update: pass OVERWRITING_FAIL Date: Thu, 30 Apr 2020 06:25:05 -0400 Message-Id: X-Mailer: git-send-email 2.26.2.548.gbb00c8a0a9 In-Reply-To: <22eacd20a8213c3281974a6c103fd2d00c95efaa.1588162842.git.liu.denton@gmail.com> References: <22eacd20a8213c3281974a6c103fd2d00c95efaa.1588162842.git.liu.denton@gmail.com> MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We are using `test_must_fail $command`. However, $command is not necessarily a git command; it could be a test helper function. In an effort to stop using test_must_fail with non-git commands, instead of invoking `test_must_fail $command`, run `OVERWRITING_FAIL=test_must_fail $command` instead. Increase the granularity of the test helper functions by specifically choosing the individual git invocation which is designed to fail. This is useful because currently, when we run a test helper function, we just mark the whole thing as `test_must_fail`. However, it's possible that the helper function might fail earlier or later than expected due to an introduced bug. If this happens, then the test case will still report as passing but it should really be marked as failing since it didn't actually display the intended behaviour. While we're at it, some helper functions have git commands piping into another git command. Break these pipes up into two separate invocations 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`. Signed-off-by: Denton Liu --- Range-diff against v1: 1: 22eacd20a8 ! 1: f0d8dcf7dc lib-submodule-update: pass OVERWRITING_FAIL @@ Commit message granularity of the test helper functions by specifically choosing the individual git invocation which is designed to fail. + This is useful because currently, when we run a test helper function, we + just mark the whole thing as `test_must_fail`. However, it's possible + that the helper function might fail earlier or later than expected due + to an introduced bug. If this happens, then the test case will still + report as passing but it should really be marked as failing since it + didn't actually display the intended behaviour. + While we're at it, some helper functions have git commands piping into another git command. Break these pipes up into two separate invocations with a file buffer so that the return code of the first command is not @@ t/lib-submodule-update.sh: test_submodule_switch_common () { git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory && - test_must_fail $command replace_sub1_with_directory && + OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory && ++ OVERWRITING_FAIL= && test_superproject_content origin/add_sub1 && test_submodule_content sub1 origin/add_sub1 ) t/lib-submodule-update.sh | 26 ++++++++++++++++++-------- t/t3426-rebase-submodule.sh | 4 ++-- t/t3513-revert-submodule.sh | 20 ++++++++++++-------- t/t3906-stash-submodule.sh | 15 +++++++++------ t/t4137-apply-submodule.sh | 6 ++++-- t/t4255-am-submodule.sh | 6 ++++-- t/t5572-pull-submodule.sh | 8 ++++---- t/t6041-bisect-submodule.sh | 33 ++++++++++++++++++--------------- 8 files changed, 71 insertions(+), 47 deletions(-) diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh index cd80d77707..f61c3b43df 100644 --- a/t/lib-submodule-update.sh +++ b/t/lib-submodule-update.sh @@ -304,12 +304,15 @@ test_submodule_content () { # a removed submodule. # # Removing a submodule containing a .git directory must fail even when forced -# to protect the history! +# to protect the history! If we are testing this case, +# OVERWRITING_FAIL=test_must_fail, otherwise OVERWRITING_FAIL will be the empty +# string. # # Internal function; use test_submodule_switch_func(), test_submodule_switch_func(), # or test_submodule_forced_switch() instead. test_submodule_switch_common () { + OVERWRITING_FAIL= command="$1" ######################### Appearing submodule ######################### # Switching to a commit letting a submodule appear creates empty dir ... @@ -443,7 +446,8 @@ test_submodule_switch_common () { ( cd submodule_update && git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory && - test_must_fail $command replace_sub1_with_directory && + OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory && + OVERWRITING_FAIL= && test_superproject_content origin/add_sub1 && test_submodule_content sub1 origin/add_sub1 ) @@ -456,7 +460,7 @@ test_submodule_switch_common () { cd submodule_update && git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory && replace_gitfile_with_git_dir sub1 && - test_must_fail $command replace_sub1_with_directory && + OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory && test_superproject_content origin/add_sub1 && test_git_directory_is_unchanged sub1 && test_submodule_content sub1 origin/add_sub1 @@ -470,7 +474,7 @@ test_submodule_switch_common () { ( cd submodule_update && git branch -t replace_sub1_with_file origin/replace_sub1_with_file && - test_must_fail $command replace_sub1_with_file && + OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_file && test_superproject_content origin/add_sub1 && test_submodule_content sub1 origin/add_sub1 ) @@ -484,7 +488,7 @@ test_submodule_switch_common () { cd submodule_update && git branch -t replace_sub1_with_file origin/replace_sub1_with_file && replace_gitfile_with_git_dir sub1 && - test_must_fail $command replace_sub1_with_file && + OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_file && test_superproject_content origin/add_sub1 && test_git_directory_is_unchanged sub1 && test_submodule_content sub1 origin/add_sub1 @@ -559,6 +563,11 @@ test_submodule_switch_common () { # conditions, set the appropriate KNOWN_FAILURE_* variable used in the tests # below to 1. # +# Removing a submodule containing a .git directory must fail even when forced +# to protect the history! If we are testing this case, +# OVERWRITING_FAIL=test_must_fail, otherwise OVERWRITING_FAIL will be the empty +# string. +# # Use as follows: # # my_func () { @@ -568,6 +577,7 @@ test_submodule_switch_common () { # } # test_submodule_switch_func "my_func" test_submodule_switch_func () { + OVERWRITING_FAIL= command="$1" test_submodule_switch_common "$command" @@ -580,7 +590,7 @@ test_submodule_switch_func () { cd submodule_update && git branch -t add_sub1 origin/add_sub1 && >sub1 && - test_must_fail $command add_sub1 && + OVERWRITING_FAIL=test_must_fail $command add_sub1 && test_superproject_content origin/no_submodule && test_must_be_empty sub1 ) @@ -588,7 +598,7 @@ test_submodule_switch_func () { } test_submodule_switch () { - test_submodule_switch_func "git $1" + test_submodule_switch_func "eval \$OVERWRITING_FAIL git $1" } # Same as test_submodule_switch(), except that throwing away local changes in @@ -596,7 +606,7 @@ test_submodule_switch () { test_submodule_forced_switch () { command="$1" KNOWN_FAILURE_FORCED_SWITCH_TESTS=1 - test_submodule_switch_common "git $command" + test_submodule_switch_common "eval \$OVERWRITING_FAIL git $command" # When forced, a file in the superproject does not prevent creating a # submodule of the same name. diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh index 788605ccc0..c6a7f584ed 100755 --- a/t/t3426-rebase-submodule.sh +++ b/t/t3426-rebase-submodule.sh @@ -17,7 +17,7 @@ git_rebase () { git status -su >actual && ls -1pR * >>actual && test_cmp expect actual && - git rebase "$1" + $OVERWRITING_FAIL git rebase "$1" } test_submodule_switch_func "git_rebase" @@ -35,7 +35,7 @@ git_rebase_interactive () { test_cmp expect actual && set_fake_editor && echo "fake-editor.sh" >.git/info/exclude && - git rebase -i "$1" + $OVERWRITING_FAIL git rebase -i "$1" } test_submodule_switch_func "git_rebase_interactive" diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh index 95a7f64471..6c899db7e1 100755 --- a/t/t3513-revert-submodule.sh +++ b/t/t3513-revert-submodule.sh @@ -15,14 +15,18 @@ 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" + 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 diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh index 6a7e801ca0..860940072d 100755 --- a/t/t3906-stash-submodule.sh +++ b/t/t3906-stash-submodule.sh @@ -8,12 +8,15 @@ test_description='stash can handle submodules' git_stash () { 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" + 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 diff --git a/t/t4137-apply-submodule.sh b/t/t4137-apply-submodule.sh index b645e303a0..dd08d9e1a4 100755 --- a/t/t4137-apply-submodule.sh +++ b/t/t4137-apply-submodule.sh @@ -6,13 +6,15 @@ test_description='git apply handling submodules' . "$TEST_DIRECTORY"/lib-submodule-update.sh apply_index () { - git diff --ignore-submodules=dirty "..$1" | git apply --index - + git diff --ignore-submodules=dirty "..$1" >diff && + $OVERWRITING_FAIL git apply --index - diff && + $OVERWRITING_FAIL git apply --3way - patch && + $OVERWRITING_FAIL git am - patch && + $OVERWRITING_FAIL git am --3way - >expect && 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" + 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"