Re: [PATCH v5] Test fsck a bit harder

Thomas Rast [off-list ref] writes:

Johannes Sixt wrote:

quoted

This is wrong: It does not test the exist status. In a pipeline, the shell 
looks only at the exit status of the last command. You really want this as

 	test_must_fail git fsck >out 2>&1 &&

You're right.

And you still write this after the lesson?

+test_expect_success 'branch pointing to non-commit' '
+	git rev-parse HEAD^{tree} > .git/refs/heads/invalid &&
+	git fsck 2>&1 | tee out &&
+	grep "not a commit" out
+'

+git update-ref -d refs/heads/invalid
+
+cat > invalid-tag <<EOF
+object ffffffffffffffffffffffffffffffffffffffff
+type commit
+tag invalid
+tagger T A Gger [off-list ref] 1234567890 -0000
+
+This is an invalid tag.
+EOF

Technically speaking, the tag is perfectly valid.  It just points to a
commit you do not have.

+cat > wrong-tag <<EOF
+object $(echo blob | git hash-object -w --stdin)
+type commit
+tag wrong
+tagger T A Gger [off-list ref] 1234567890 -0000
+
+This is an invalid tag.
+EOF

This is indeed invalid.

+test_expect_success 'tag pointing to something else than its type' '
+	tag=$(git hash-object -t tag -w --stdin < wrong-tag) &&
+	echo $tag > .git/refs/tags/wrong &&
+	test_must_fail git fsck >out 2>&1 &&
+	grep "Object.*is a blob, not a commit" out
+'
+
+rm .git/refs/tags/wrong

I'd rather want to see test_expect_success around this "rm", and replace
that rm with something more git-ish, like "tag -d" or "update-ref -d".

By the way, is it just me or would we want to reword the subject of the
commit to tame it a little bit?