From patchwork Thu May 21 00:24:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Denton Liu X-Patchwork-Id: 11561829 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 CBFD160D for ; Thu, 21 May 2020 00:24:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A8AB8205CB for ; Thu, 21 May 2020 00:24:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="LTizk0Wz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726886AbgEUAYv (ORCPT ); Wed, 20 May 2020 20:24:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36586 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726737AbgEUAYu (ORCPT ); Wed, 20 May 2020 20:24:50 -0400 Received: from mail-qt1-x843.google.com (mail-qt1-x843.google.com [IPv6:2607:f8b0:4864:20::843]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 525D7C061A0E for ; Wed, 20 May 2020 17:24:50 -0700 (PDT) Received: by mail-qt1-x843.google.com with SMTP id t25so4224025qtc.0 for ; Wed, 20 May 2020 17:24:50 -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=H+0hlYKuSJ3bI5x+Hs+Oun0jr5jAoOyysBhudmdYTcU=; b=LTizk0WzQR5jKFH1cF6o9yK5F0LlTfxL9wG82ze6by3B46SxFAQwOAOGZTFxHgI9Bl 2oUziH9jVMmmdIHAAcodhbhTdsMJS6EQEAMaEXK2KMrPS3ozDH+uzUwZiHlC7gfPIj+u qdUyY5WjScjfjM8Hq6RGdiZ+pB6n9TtM1JlsgQISg0AVGg9pj9/KKUGuAF3uVI5dFDh9 X+OzRJywhaVla/0WiWLzJh5KTmYaG6qW/SGSSgDMpBRm/IrBG+MC64p8qCaaByRSNqP/ QistHurdz/mtBCfJk72dBCtOmHf16wjZUyd+1Fp1pP6PCUs1Z/Qhs3oy+x3LORw1rESv /WDA== 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=H+0hlYKuSJ3bI5x+Hs+Oun0jr5jAoOyysBhudmdYTcU=; b=C55lLD7gEsSrPlUACH/LHL4klhnRbM2S2cQWdrfAoyKMFQnXlITbW65+yQsSunG9XY pgVzHdiBozpHJqDHIHj4Bhv5m/UCVophzUeQgFSrE+FqeRFm7MM3IGtdM2a2aVLONPG8 TkmVSToQlSsOro4FViHRASzCj96J+kP//8d5w9F19tCFrBQMEvJrpsWUUw7P3/7Qlhra /S2kq8uv9UEeoNO7P266YoEQcxBsFuRn8FnnQXl53FJLSk6v5VOANUTkZxNBjwzOMwlu isEDhAHC/ha7mDdUd1C40j1KIrLpbrAVsxn16tPvBCWUdZUzaS6DpKLvaaD6esAs/FyX 7F4w== X-Gm-Message-State: AOAM532wBhB1Cf3FUtc+pxTO8XUk+lhuxXILACPydEVo4NY6YXIpV8nJ K9q/fEjSqVXJjla2CTh6fN61EjK3 X-Google-Smtp-Source: ABdhPJy3QFHvUElHAnqxMftf+Dnm7fuyrzKMBwwK6i8qcSsAxNIaLkcnLpTaBLCnjBjrXpc0pqLgQA== X-Received: by 2002:ac8:3782:: with SMTP id d2mr8128242qtc.74.1590020689068; Wed, 20 May 2020 17:24:49 -0700 (PDT) Received: from archbookpro.localdomain (CPE18593399858a-CM185933998587.cpe.net.cable.rogers.com. [174.112.65.113]) by smtp.gmail.com with ESMTPSA id w67sm3539023qkb.102.2020.05.20.17.24.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 May 2020 17:24:48 -0700 (PDT) From: Denton Liu To: Git Mailing List Cc: Junio C Hamano , Taylor Blau , Johannes Sixt , Eric Sunshine Subject: [PATCH v2 4/4] lib-submodule-update: pass OVERWRITING_FAIL Date: Wed, 20 May 2020 20:24:18 -0400 Message-Id: <48598e3f9859dc525ec878cd7f3eaadee8bb61b1.1590019226.git.liu.denton@gmail.com> X-Mailer: git-send-email 2.27.0.rc0.114.g141fe7d276 In-Reply-To: References: 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 in test_submodule_switch_common(). In the case where $command is a git command, test_submodule_switch_common() is called by one of test_submodule_switch() or test_submodule_forced_switch(). In those two functions, pass $command like this: test_submodule_switch_common "eval \$OVERWRITING_FAIL git $command" When $command is a helper function, the parent function calling test_submodule_switch_common() is test_submodule_switch_func(). For all test_submodule_switch_func() invocations, increase the granularity of the argument test helper function by prefixing the git invocation which is meant to fail with $OVERWRITING_FAIL like this: $OVERWRITING_FAIL git checkout "$1" 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. Signed-off-by: Denton Liu --- t/lib-submodule-update.sh | 33 +++++++++++++++++++++++++-------- t/t3426-rebase-submodule.sh | 4 ++-- t/t3513-revert-submodule.sh | 5 +++++ t/t3906-stash-submodule.sh | 6 +++++- t/t4137-apply-submodule.sh | 6 ++++-- t/t4255-am-submodule.sh | 6 ++++-- t/t5572-pull-submodule.sh | 8 ++++---- t/t6041-bisect-submodule.sh | 6 +++++- 8 files changed, 54 insertions(+), 20 deletions(-) diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh index fb6c0f3d4f..a0274c03de 100755 --- 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,9 @@ 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 +461,9 @@ 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 && + OVERWRITING_FAIL= && test_superproject_content origin/add_sub1 && test_git_directory_is_unchanged sub1 && test_submodule_content sub1 origin/add_sub1 @@ -470,7 +477,9 @@ 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 && + OVERWRITING_FAIL= && test_superproject_content origin/add_sub1 && test_submodule_content sub1 origin/add_sub1 ) @@ -484,7 +493,9 @@ 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 && + OVERWRITING_FAIL= && test_superproject_content origin/add_sub1 && test_git_directory_is_unchanged sub1 && test_submodule_content sub1 origin/add_sub1 @@ -559,6 +570,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 +584,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 +597,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 +605,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 +613,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..e41f0f2009 100755 --- a/t/t3513-revert-submodule.sh +++ b/t/t3513-revert-submodule.sh @@ -15,6 +15,11 @@ git_revert () { git status -su >expect && ls -1pR * >>expect && tar cf "$TRASH_DIRECTORY/tmp.tar" * && + $OVERWRITING_FAIL git checkout "$1" && + if test -n "$OVERWRITING_FAIL" + then + return + fi && git checkout "$1" && git revert HEAD && rm -rf * && diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh index 6a7e801ca0..6e8dac9669 100755 --- a/t/t3906-stash-submodule.sh +++ b/t/t3906-stash-submodule.sh @@ -8,7 +8,11 @@ test_description='stash can handle submodules' git_stash () { git status -su >expect && ls -1pR * >>expect && - git read-tree -u -m "$1" && + $OVERWRITING_FAIL git read-tree -u -m "$1" && + if test -n "$OVERWRITING_FAIL" + then + return + fi && git stash && git status -su >actual && ls -1pR * >>actual && 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" && + $OVERWRITING_FAIL git checkout "$1" && + if test -n "$OVERWRITING_FAIL" + then + return + fi && echo "foo" >bar && git add bar && git commit -m "bisect bad" &&