Thread (15 messages) 15 messages, 4 authors, 2025-08-26

Re: [PATCH 1/3] t7700: add failing --path-walk test

From: Derrick Stolee <hidden>
Date: 2025-08-21 12:43:21

On 8/21/2025 4:00 AM, Patrick Steinhardt wrote:
On Wed, Aug 20, 2025 at 06:39:54PM +0000, Derrick Stolee via GitGitGadget wrote:
quoted
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 611755cc139b..1998d9bf291c 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -838,4 +838,47 @@ test_expect_success '-n overrides repack.updateServerInfo=true' '
Tiny nit: I would've probably squashed this patch into the second patch,
as we usually don't use the add-failing-test-and-then-fix-it-later
dance. On the other hand though it gives some nice context, so I
ultimately don't mind it all that much. So please feel free to ignore
this nit.
I'm probably the person who is always asking folks to create a test
that either fails or demonstrates the "before" behavior before making
the actual change that updates the case. This allows the ability to
"test the test" by checking it in place to confirm that it is indeed
failing.

Using test_expect_failure allows us to avoid breaking bisect. 
quoted
 	test_server_info_missing
 '
 
+test_expect_failure 'pending objects are repacked appropriately' '
+	git init pending &&
We probably also want `test_when_finished "rm -rf pending"` before
calling git-init(1).
Good idea.
quoted
+
+	(
+		cd pending &&
+
+		mkdir -p a/b &&
+		echo singleton >file &&
+		echo stuff >a/b/c &&
+		echo more >a/d &&
+		git add file a &&
+		git commit -m "single blobs" &&
+
+		echo d >a/d &&
+		echo e >a/e &&
+		git add a &&
+		git commit -m "more blobs" &&
+
+		# This use of a sparse index helps to force
+		# test that the cache-tree is walked, too.
+		git sparse-checkout set --sparse-index a x &&
+
+		# Just _stage_ the changes.
+		echo f >a/d &&
+		echo h >a/e &&
+		echo i >a/i &&
+		mkdir x &&
+		echo y >x/y &&
+		git add a x &&
Nit: I think I would've moved the explanations you have in the commit
message into these hunks so that the test becomes a bit more
self-explanatory.
Hm. That seems to go against our typical pattern of leaving comments
sparse and having the longer explanation available in commit messages
but maybe I'm out of date or tests are a different beast. I'll see
what I can do to make the test more self-documented.

Thanks,
-Stolee
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help