threads / patch / 61566

patcht1517: more coverage for commands that work without repository

Subject: [PATCH] t1517: more coverage for commands that work without repository

## tl;dr

4 messages between May 30, 2024 and May 31, 2024. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Junio C Hamano· May 30, 2024, 16:58 UTC · lore

While most of the commands in Git suite are designed to do useful things in Git repositories, some commands are also usable outside any repository. Building on top of an earlier work abece6e9 (t1517: test commands that are designed to be run outside repository, 2024-05-20) that adds tests for such commands, let's give coverage to some more commands.

This patch covers commands whose code has hits for
    $ git grep setup_git_directory_gently

and passes a pointer to nongit_ok variable it uses to allow it to run outside a Git repository, but mostly they are tested only to see that they start up (as opposed to dying with "not in a git repository" complaint). We may want to update them to actually do something useful later, but this would at least help us catch regressions by mistake.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Designed to apply on top of jc/undecided-is-not-necessarily-sha1-fix
   that introduces t1517.
 t/t1517-outside-repo.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
Show changes to diff +48 −0
diff --git c/t/t1517-outside-repo.sh w/t/t1517-outside-repo.sh
index 557808ffa7..b77017ac5f 100755
--- c/t/t1517-outside-repo.sh
+++ w/t/t1517-outside-repo.sh
@@ -56,4 +56,52 @@ test_expect_success 'grep outside repository' '
 	test_cmp expect actual
 '
 
+test_expect_success 'imap-send outside repository' '
+	test_config_global imap.host imaps://localhost &&
+	test_config_global imap.folder Drafts &&
+
+	echo nothing to send >expect &&
+	test_must_fail git imap-send -v </dev/null 2>actual &&
+	test_cmp expect actual &&
+
+	(
+		cd non-repo &&
+		test_must_fail git imap-send -v </dev/null 2>../actual
+	) &&
+	test_cmp expect actual
+'
+
+test_expect_success 'check-ref-format outside repository' '
+	git check-ref-format --branch refs/heads/xyzzy >expect &&
+	nongit git check-ref-format --branch refs/heads/xyzzy >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff outside repository' '
+	echo one >one &&
+	echo two >two &&
+	test_must_fail git diff --no-index one two >expect &&
+	(
+		cd non-repo &&
+		cp ../one . &&
+		cp ../two . &&
+		test_must_fail git diff one two >../actual
+	) &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stripspace outside repository' '
+	nongit git stripspace -s </dev/null
+'
+
+test_expect_success 'remote-http outside repository' '
+	test_must_fail git remote-http 2>actual &&
+	test_grep "^error: remote-curl" actual &&
+	(
+		cd non-repo &&
+		test_must_fail git remote-http 2>../actual
+	) &&
+	test_grep "^error: remote-curl" actual
+'
+
 test_done
Junio C Hamano· May 31, 2024, 08:43 UTC · re: Junio C Hamano · lore

[PATCH v2] t1517: more coverage for commands that work without repository

While most of the commands in Git suite are designed to do useful things in Git repositories, some commands are also usable outside any repository. Building on top of an earlier work abece6e9 (t1517: test commands that are designed to be run outside repository, 2024-05-20) that adds tests for such commands, let's give coverage to some more commands.

This patch covers commands whose code has hits for
    $ git grep setup_git_directory_gently

and passes a pointer to nongit_ok variable it uses to allow it to run outside a Git repository, but mostly they are tested only to see that they start up (as opposed to dying with "not in a git repository" complaint). We may want to update them to actually do something useful later, but this would at least help us catch regressions by mistake.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * The previous iteration was buggy when the repository was using
   sha256, as the nongit invocation of "git diff" will use sha1.
 t/t1517-outside-repo.sh | 52 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)
Show changes to t/t1517-outside-repo.sh +52 −0
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index 557808ffa7..990a036582 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -56,4 +56,56 @@ test_expect_success 'grep outside repository' '
 	test_cmp expect actual
 '
 
+test_expect_success 'imap-send outside repository' '
+	test_config_global imap.host imaps://localhost &&
+	test_config_global imap.folder Drafts &&
+
+	echo nothing to send >expect &&
+	test_must_fail git imap-send -v </dev/null 2>actual &&
+	test_cmp expect actual &&
+
+	(
+		cd non-repo &&
+		test_must_fail git imap-send -v </dev/null 2>../actual
+	) &&
+	test_cmp expect actual
+'
+
+test_expect_success 'check-ref-format outside repository' '
+	git check-ref-format --branch refs/heads/xyzzy >expect &&
+	nongit git check-ref-format --branch refs/heads/xyzzy >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff outside repository' '
+	echo one >one &&
+	echo two >two &&
+	test_must_fail git diff --no-index one two >expect.raw &&
+	(
+		cd non-repo &&
+		cp ../one . &&
+		cp ../two . &&
+		test_must_fail git diff one two >../actual.raw
+	) &&
+	# outside repository diff falls back to SHA-1 but
+	# GIT_DEFAULT_HASH may be set to sha256 on the in-repo side.
+	sed -e "/^index /d" expect.raw >expect &&
+	sed -e "/^index /d" actual.raw >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stripspace outside repository' '
+	nongit git stripspace -s </dev/null
+'
+
+test_expect_success 'remote-http outside repository' '
+	test_must_fail git remote-http 2>actual &&
+	test_grep "^error: remote-curl" actual &&
+	(
+		cd non-repo &&
+		test_must_fail git remote-http 2>../actual
+	) &&
+	test_grep "^error: remote-curl" actual
+'
+
 test_done

Interdiff against v1:
  diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
  index b77017ac5f..990a036582 100755
  --- a/t/t1517-outside-repo.sh
  +++ b/t/t1517-outside-repo.sh
  @@ -80,13 +80,17 @@ test_expect_success 'check-ref-format outside repository' '
   test_expect_success 'diff outside repository' '
   	echo one >one &&
   	echo two >two &&
  -	test_must_fail git diff --no-index one two >expect &&
  +	test_must_fail git diff --no-index one two >expect.raw &&
   	(
   		cd non-repo &&
   		cp ../one . &&
   		cp ../two . &&
  -		test_must_fail git diff one two >../actual
  +		test_must_fail git diff one two >../actual.raw
   	) &&
  +	# outside repository diff falls back to SHA-1 but
  +	# GIT_DEFAULT_HASH may be set to sha256 on the in-repo side.
  +	sed -e "/^index /d" expect.raw >expect &&
  +	sed -e "/^index /d" actual.raw >actual &&
   	test_cmp expect actual
   '
   

-- 
2.45.2-584-g20241e85ff
Patrick Steinhardt· May 31, 2024, 11:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] t1517: more coverage for commands that work without repository

On Thu, May 30, 2024 at 09:58:46AM -0700, Junio C Hamano wrote: [snip]

> +test_expect_success 'stripspace outside repository' '
> +	nongit git stripspace -s </dev/null
> +'

I was wondering whether we also want to verify that it does the correct thing here as it's comparatively easy:

	test_expect_success 'stripspace outside repository' '
		cat >input <<-EOF &&
		# foo
		bar
		EOF
		cat >expect <<-EOF &&
		bar
		EOF
		git stripspace -s <input >actual &&
		test_cmp expect actual &&
		nongit git stripspace -s <input >actual &&
		test_cmp expect actual
	'

For the other tests it's a bit more involved. And given that these are all designed as smoke tests, I think it's fine overall.

Thanks!
Patrick
Junio C Hamano· May 31, 2024, 14:57 UTC · re: Patrick Steinhardt · lore

Re: [PATCH] t1517: more coverage for commands that work without repository

Patrick Steinhardt <ps@pks.im> writes:
Show 11 quoted lines
> On Thu, May 30, 2024 at 09:58:46AM -0700, Junio C Hamano wrote:
> [snip]
>> +test_expect_success 'stripspace outside repository' '
>> +	nongit git stripspace -s </dev/null
>> +'
>
> I was wondering whether we also want to verify that it does the correct
> thing here as it's comparatively easy:
> ...
> For the other tests it's a bit more involved. And given that these are
> all designed as smoke tests, I think it's fine overall.

Yup, as I said in the proposed log message, we may want to update them to actually do something useful later, but this would at least help us catch regressions by mistake.

← back to recent threads