From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwen@google.com>
Subject: Re: [PATCH 03/11] t: convert tests to use helpers for reference existence
Date: Wed, 18 Oct 2023 09:28:50 -0700 [thread overview]
Message-ID: <xmqqcyxbuc71.fsf@gitster.g> (raw)
In-Reply-To: <ac6a49c7c84ad2b836f099557dd6989703ebda8f.1697607222.git.ps@pks.im> (Patrick Steinhardt's message of "Wed, 18 Oct 2023 07:35:16 +0200")
Patrick Steinhardt <ps@pks.im> writes:
> Convert tests that use `test_path_is_file` and `test_path_is_missing` to
> instead use our new helpers `test_ref_exists` and `test_ref_missing`.
> These hook into the reference database directly and thus work indepently
> of the actual reference backend that is being used.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> t/t1430-bad-ref-name.sh | 27 ++++++++++++++++++---------
> t/t3200-branch.sh | 33 ++++++++++++++++++---------------
> t/t5521-pull-options.sh | 4 ++--
> t/t5605-clone-local.sh | 2 +-
> 4 files changed, 39 insertions(+), 27 deletions(-)
I scanned through the changes, and all of them looked sensible. I
also very much liked the added "now we have done something to create
'badname' branch, let's make sure it exists, before trying to see if
the machinery to delete it works correctly" tests.
Looking good.
Thanks.
> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> index ff1c967d550..7b7d6953c62 100755
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -205,8 +205,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git update-ref --no-deref -d refs/heads/badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -216,8 +217,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git branch -d badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
> test_must_be_empty error
> '
> @@ -225,8 +227,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
> test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git update-ref --no-deref -d refs/heads/badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -234,8 +237,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok
> test_expect_success 'branch -d can delete dangling symref to broken name' '
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/badname &&
> git branch -d badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/badname &&
> + test_ref_missing refs/heads/badname &&
> test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
> test_must_be_empty error
> '
> @@ -245,8 +249,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
> test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
> + test_ref_exists refs/heads/broken...ref &&
> git update-ref -d refs/heads/badname >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...ref &&
> + test_ref_missing refs/heads/broken...ref &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -254,8 +259,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
> test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
> printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -263,8 +269,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name
> test_expect_success 'branch -d can delete symref with broken name' '
> printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git branch -d broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
> test_must_be_empty error
> '
> @@ -272,8 +279,9 @@ test_expect_success 'branch -d can delete symref with broken name' '
> test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
> printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_must_be_empty output &&
> test_must_be_empty error
> '
> @@ -281,8 +289,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
> test_expect_success 'branch -d can delete dangling symref with broken name' '
> printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
> test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
> + test_ref_exists refs/heads/broken...symref &&
> git branch -d broken...symref >output 2>error &&
> - test_path_is_missing .git/refs/heads/broken...symref &&
> + test_ref_missing refs/heads/broken...symref &&
> test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
> test_must_be_empty error
> '
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 080e4f24a6e..bde4f1485b7 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -25,7 +25,7 @@ test_expect_success 'prepare a trivial repository' '
>
> test_expect_success 'git branch --help should not have created a bogus branch' '
> test_might_fail git branch --man --help </dev/null >/dev/null 2>&1 &&
> - test_path_is_missing .git/refs/heads/--help
> + test_ref_missing refs/heads/--help
> '
>
> test_expect_success 'branch -h in broken repository' '
> @@ -40,7 +40,8 @@ test_expect_success 'branch -h in broken repository' '
> '
>
> test_expect_success 'git branch abc should create a branch' '
> - git branch abc && test_path_is_file .git/refs/heads/abc
> + git branch abc &&
> + test_ref_exists refs/heads/abc
> '
>
> test_expect_success 'git branch abc should fail when abc exists' '
> @@ -61,11 +62,13 @@ test_expect_success 'git branch --force abc should succeed when abc exists' '
> '
>
> test_expect_success 'git branch a/b/c should create a branch' '
> - git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c
> + git branch a/b/c &&
> + test_ref_exists refs/heads/a/b/c
> '
>
> test_expect_success 'git branch mb main... should create a branch' '
> - git branch mb main... && test_path_is_file .git/refs/heads/mb
> + git branch mb main... &&
> + test_ref_exists refs/heads/mb
> '
>
> test_expect_success 'git branch HEAD should fail' '
> @@ -78,14 +81,14 @@ EOF
> test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
> GIT_COMMITTER_DATE="2005-05-26 23:30" \
> git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
> - test_path_is_file .git/refs/heads/d/e/f &&
> + test_ref_exists refs/heads/d/e/f &&
> test_path_is_file .git/logs/refs/heads/d/e/f &&
> test_cmp expect .git/logs/refs/heads/d/e/f
> '
>
> test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
> git branch -d d/e/f &&
> - test_path_is_missing .git/refs/heads/d/e/f &&
> + test_ref_missing refs/heads/d/e/f &&
> test_must_fail git reflog exists refs/heads/d/e/f
> '
>
> @@ -213,7 +216,7 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
> test_commit initial &&
> git checkout --orphan lonely &&
> grep lonely .git/HEAD &&
> - test_path_is_missing .git/refs/head/lonely &&
> + test_ref_missing refs/head/lonely &&
> git branch -M main mistress &&
> grep lonely .git/HEAD
> )
> @@ -799,8 +802,8 @@ test_expect_success 'deleting a symref' '
> git symbolic-ref refs/heads/symref refs/heads/target &&
> echo "Deleted branch symref (was refs/heads/target)." >expect &&
> git branch -d symref >actual &&
> - test_path_is_file .git/refs/heads/target &&
> - test_path_is_missing .git/refs/heads/symref &&
> + test_ref_exists refs/heads/target &&
> + test_ref_missing refs/heads/symref &&
> test_cmp expect actual
> '
>
> @@ -809,16 +812,16 @@ test_expect_success 'deleting a dangling symref' '
> test_path_is_file .git/refs/heads/dangling-symref &&
> echo "Deleted branch dangling-symref (was nowhere)." >expect &&
> git branch -d dangling-symref >actual &&
> - test_path_is_missing .git/refs/heads/dangling-symref &&
> + test_ref_missing refs/heads/dangling-symref &&
> test_cmp expect actual
> '
>
> test_expect_success 'deleting a self-referential symref' '
> git symbolic-ref refs/heads/self-reference refs/heads/self-reference &&
> - test_path_is_file .git/refs/heads/self-reference &&
> + test_ref_exists refs/heads/self-reference &&
> echo "Deleted branch self-reference (was refs/heads/self-reference)." >expect &&
> git branch -d self-reference >actual &&
> - test_path_is_missing .git/refs/heads/self-reference &&
> + test_ref_missing refs/heads/self-reference &&
> test_cmp expect actual
> '
>
> @@ -826,8 +829,8 @@ test_expect_success 'renaming a symref is not allowed' '
> git symbolic-ref refs/heads/topic refs/heads/main &&
> test_must_fail git branch -m topic new-topic &&
> git symbolic-ref refs/heads/topic &&
> - test_path_is_file .git/refs/heads/main &&
> - test_path_is_missing .git/refs/heads/new-topic
> + test_ref_exists refs/heads/main &&
> + test_ref_missing refs/heads/new-topic
> '
>
> test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
> @@ -1142,7 +1145,7 @@ EOF
> test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
> GIT_COMMITTER_DATE="2005-05-26 23:30" \
> git checkout -b g/h/i -l main &&
> - test_path_is_file .git/refs/heads/g/h/i &&
> + test_ref_exists refs/heads/g/h/i &&
> test_path_is_file .git/logs/refs/heads/g/h/i &&
> test_cmp expect .git/logs/refs/heads/g/h/i
> '
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index 079b2f2536e..3681859f983 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -143,7 +143,7 @@ test_expect_success 'git pull --dry-run' '
> cd clonedry &&
> git pull --dry-run ../parent &&
> test_path_is_missing .git/FETCH_HEAD &&
> - test_path_is_missing .git/refs/heads/main &&
> + test_ref_missing refs/heads/main &&
> test_path_is_missing .git/index &&
> test_path_is_missing file
> )
> @@ -157,7 +157,7 @@ test_expect_success 'git pull --all --dry-run' '
> git remote add origin ../parent &&
> git pull --all --dry-run &&
> test_path_is_missing .git/FETCH_HEAD &&
> - test_path_is_missing .git/refs/remotes/origin/main &&
> + test_ref_missing refs/remotes/origin/main &&
> test_path_is_missing .git/index &&
> test_path_is_missing file
> )
> diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
> index 1d7b1abda1a..946c5751885 100755
> --- a/t/t5605-clone-local.sh
> +++ b/t/t5605-clone-local.sh
> @@ -69,7 +69,7 @@ test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
> git clone a d &&
> (cd d &&
> git fetch &&
> - test ! -e .git/refs/remotes/origin/HEAD)
> + test_ref_missing refs/remotes/origin/HEAD)
> '
>
> test_expect_success 'bundle clone without .bundle suffix' '
next prev parent reply other threads:[~2023-10-18 16:29 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-18 5:35 [PATCH 00/11] t: reduce direct disk access to data structures Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 01/11] t: add helpers to test for reference existence Patrick Steinhardt
2023-10-18 16:06 ` Junio C Hamano
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-18 17:08 ` Eric Sunshine
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 02/11] t: allow skipping expected object ID in `ref-store update-ref` Patrick Steinhardt
2023-10-18 16:08 ` Junio C Hamano
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-23 19:06 ` Junio C Hamano
2023-10-18 5:35 ` [PATCH 03/11] t: convert tests to use helpers for reference existence Patrick Steinhardt
2023-10-18 16:28 ` Junio C Hamano [this message]
2023-10-18 5:35 ` [PATCH 04/11] t: convert tests to not write references via the filesystem Patrick Steinhardt
2023-10-18 18:34 ` Junio C Hamano
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-23 19:10 ` Junio C Hamano
2023-10-18 21:18 ` Junio C Hamano
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 05/11] t: convert tests to not access symrefs " Patrick Steinhardt
2023-10-20 19:52 ` Junio C Hamano
2023-10-18 5:35 ` [PATCH 06/11] t: convert tests to not access reflog " Patrick Steinhardt
2023-10-21 23:13 ` Junio C Hamano
2023-10-18 5:35 ` [PATCH 07/11] t1450: convert tests to remove worktrees via git-worktree(1) Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 08/11] t4207: delete replace references via git-update-ref(1) Patrick Steinhardt
2023-10-18 13:27 ` Han-Wen Nienhuys
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-23 16:42 ` Taylor Blau
2023-10-24 6:42 ` Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 09/11] t7300: assert exact states of repo Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 10/11] t7900: assert the absence of refs via git-for-each-ref(1) Patrick Steinhardt
2023-10-18 5:35 ` [PATCH 11/11] t: mark several tests that assume the files backend with REFFILES Patrick Steinhardt
2023-10-18 5:39 ` [PATCH 00/11] t: reduce direct disk access to data structures Patrick Steinhardt
2023-10-18 23:40 ` Junio C Hamano
2023-10-23 11:57 ` Patrick Steinhardt
2023-10-18 15:32 ` Junio C Hamano
2023-10-19 10:13 ` Han-Wen Nienhuys
2023-10-19 17:55 ` Junio C Hamano
2023-10-23 13:58 ` Patrick Steinhardt
2023-10-24 14:04 ` [PATCH v2 0/9] " Patrick Steinhardt
2023-10-24 14:04 ` [PATCH v2 1/9] t: allow skipping expected object ID in `ref-store update-ref` Patrick Steinhardt
2023-10-24 14:04 ` [PATCH v2 2/9] t: convert tests to not write references via the filesystem Patrick Steinhardt
2023-10-24 14:05 ` [PATCH v2 3/9] t: convert tests to not access symrefs " Patrick Steinhardt
2023-10-24 14:05 ` [PATCH v2 4/9] t: convert tests to not access reflog " Patrick Steinhardt
2023-10-24 14:05 ` [PATCH v2 5/9] t1450: convert tests to remove worktrees via git-worktree(1) Patrick Steinhardt
2023-10-27 2:42 ` Eric Sunshine
2023-10-24 14:05 ` [PATCH v2 6/9] t4207: delete replace references via git-update-ref(1) Patrick Steinhardt
2023-10-24 14:05 ` [PATCH v2 7/9] t7300: assert exact states of repo Patrick Steinhardt
2023-10-24 14:05 ` [PATCH v2 8/9] t7900: assert the absence of refs via git-for-each-ref(1) Patrick Steinhardt
2023-10-24 14:05 ` [PATCH v2 9/9] t: mark several tests that assume the files backend with REFFILES Patrick Steinhardt
2023-11-02 8:46 ` [PATCH v3 0/9] t: reduce direct disk access to data structures Patrick Steinhardt
2023-11-02 8:46 ` [PATCH v3 1/9] t: allow skipping expected object ID in `ref-store update-ref` Patrick Steinhardt
2023-11-02 8:46 ` [PATCH v3 2/9] t: convert tests to not write references via the filesystem Patrick Steinhardt
2023-11-02 8:46 ` [PATCH v3 3/9] t: convert tests to not access symrefs " Patrick Steinhardt
2023-11-02 8:46 ` [PATCH v3 4/9] t: convert tests to not access reflog " Patrick Steinhardt
2023-11-02 8:46 ` [PATCH v3 5/9] t1450: convert tests to remove worktrees via git-worktree(1) Patrick Steinhardt
2023-11-02 8:47 ` [PATCH v3 6/9] t4207: delete replace references via git-update-ref(1) Patrick Steinhardt
2023-11-02 8:47 ` [PATCH v3 7/9] t7300: assert exact states of repo Patrick Steinhardt
2023-11-02 8:47 ` [PATCH v3 8/9] t7900: assert the absence of refs via git-for-each-ref(1) Patrick Steinhardt
2023-11-02 8:47 ` [PATCH v3 9/9] t: mark several tests that assume the files backend with REFFILES Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqcyxbuc71.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=hanwen@google.com \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).