diff mbox series

[1/5] fetch test: use more robust test for filtered objects

Message ID 20191224005907.GD38316@google.com (mailing list archive)
State New, archived
Headers show
Series Enable protocol v2 by default | expand

Commit Message

Jonathan Nieder Dec. 24, 2019, 12:59 a.m. UTC
"git cat-file -e" uses has_object_file, which can fetch from promisor
remotes when an object is missing.  These tests end up checking that
that fetch fails instead of for the object being missing.

By luck, the tests pass anyway:

- in one of these tests ("filtering by size"), the fetch fails because
  (in protocol v0) the server does not support fetches by SHA-1

- in the second, the object is present but the test could pass even if
  it weren't if the fetch succeeds

- in the third, the test sets extensions.partialClone to "arbitrary
  string" so that when it tries to fetch, it looks up the "arbitrary
  string" remote which does not exist

Use "git rev-list --objects --missing=allow-any", so that the tests
pass for the right reason.

Noticed while testing with protocol v2, which allows fetching by sha1
by default, causing the first fetch to succeed and the test to fail.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t5500-fetch-pack.sh | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Derrick Stolee Dec. 26, 2019, 2:29 p.m. UTC | #1
On 12/23/2019 7:59 PM, Jonathan Nieder wrote:
>  	# Ensure that object is not inadvertently fetched
> -	test_must_fail git -C client cat-file -e $(git hash-object server/one.t)
> +	commit=$(git -C server rev-parse HEAD) &&
> +	blob=$(git hash-object server/one.t) &&
> +	git -C client rev-list --objects --missing=allow-any "$commit" >oids &&
> +	! grep "$blob" oids
>  '
>  
>  test_expect_success 'filtering by size has no effect if support for it is not advertised' '
> @@ -929,7 +932,10 @@ test_expect_success 'filtering by size has no effect if support for it is not ad
>  	git -C client fetch-pack --filter=blob:limit=0 ../server HEAD 2> err &&
>  
>  	# Ensure that object is fetched
> -	git -C client cat-file -e $(git hash-object server/one.t) &&
> +	commit=$(git -C server rev-parse HEAD) &&
> +	blob=$(git hash-object server/one.t) &&
> +	git -C client rev-list --objects --missing=allow-any "$commit" >oids &&
> +	grep "$blob" oids &&
>  
>  	test_i18ngrep "filtering not recognized by server" err
>  '
> @@ -951,9 +957,11 @@ fetch_filter_blob_limit_zero () {
>  	git -C client fetch --filter=blob:limit=0 origin HEAD:somewhere &&
>  
>  	# Ensure that commit is fetched, but blob is not
> -	test_config -C client extensions.partialclone "arbitrary string" &&
> -	git -C client cat-file -e $(git -C "$SERVER" rev-parse two) &&
> -	test_must_fail git -C client cat-file -e $(git hash-object "$SERVER/two.t")
> +	commit=$(git -C "$SERVER" rev-parse two) &&
> +	blob=$(git hash-object server/two.t) &&
> +	git -C client rev-list --objects --missing=allow-any "$commit" >oids &&
> +	grep "$commit" oids &&
> +	! grep "$blob" oids

At first glance, I saw this and thought the "three is many" rule would imply
the boilerplate in these tests should be de-duplicated with a function.
However, the use of the "commit" and "blob" variables is different in each
test, so I did not find a convenient way to make this simpler.

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 6b97923964..964930b2d2 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -917,7 +917,10 @@  test_expect_success 'filtering by size' '
 	git -C client fetch-pack --filter=blob:limit=0 ../server HEAD &&
 
 	# Ensure that object is not inadvertently fetched
-	test_must_fail git -C client cat-file -e $(git hash-object server/one.t)
+	commit=$(git -C server rev-parse HEAD) &&
+	blob=$(git hash-object server/one.t) &&
+	git -C client rev-list --objects --missing=allow-any "$commit" >oids &&
+	! grep "$blob" oids
 '
 
 test_expect_success 'filtering by size has no effect if support for it is not advertised' '
@@ -929,7 +932,10 @@  test_expect_success 'filtering by size has no effect if support for it is not ad
 	git -C client fetch-pack --filter=blob:limit=0 ../server HEAD 2> err &&
 
 	# Ensure that object is fetched
-	git -C client cat-file -e $(git hash-object server/one.t) &&
+	commit=$(git -C server rev-parse HEAD) &&
+	blob=$(git hash-object server/one.t) &&
+	git -C client rev-list --objects --missing=allow-any "$commit" >oids &&
+	grep "$blob" oids &&
 
 	test_i18ngrep "filtering not recognized by server" err
 '
@@ -951,9 +957,11 @@  fetch_filter_blob_limit_zero () {
 	git -C client fetch --filter=blob:limit=0 origin HEAD:somewhere &&
 
 	# Ensure that commit is fetched, but blob is not
-	test_config -C client extensions.partialclone "arbitrary string" &&
-	git -C client cat-file -e $(git -C "$SERVER" rev-parse two) &&
-	test_must_fail git -C client cat-file -e $(git hash-object "$SERVER/two.t")
+	commit=$(git -C "$SERVER" rev-parse two) &&
+	blob=$(git hash-object server/two.t) &&
+	git -C client rev-list --objects --missing=allow-any "$commit" >oids &&
+	grep "$commit" oids &&
+	! grep "$blob" oids
 }
 
 test_expect_success 'fetch with --filter=blob:limit=0' '