Skip to contentSkip to conversation

t1450: add tests for NUL in headers of commits and tags

PATCH In review René Scharfe 6 participants 14 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 t1450: add tests for NUL in headers of commits and tags

---
 t/t1450-fsck.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
t/t1450-fsck.sh +32 -0
index dc09797..6c96953 100755
@@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' '
 	grep "error in commit $new.*integer overflow" out
 '
 
+test_expect_success 'commit with NUL in header' '
+	git cat-file commit HEAD >basis &&
+	sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
+	new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
+	test_when_finished "remove_object $new" &&
+	git update-ref refs/heads/bogus "$new" &&
+	test_when_finished "git update-ref -d refs/heads/bogus" &&
+	test_must_fail git fsck 2>out &&
+	cat out &&
+	grep "error in commit $new.*unterminated header: NUL at offset" out
+'
+
 test_expect_success 'malformatted tree object' '
 	test_when_finished "git update-ref -d refs/tags/wrong" &&
 	test_when_finished "remove_object \$T" &&
@@ -276,6 +288,26 @@ test_expect_success 'tag with bad tagger' '
 	grep "error in tag .*: invalid author/committer" out
 '
 
+test_expect_failure 'tag with NUL in header' '
+	sha=$(git rev-parse HEAD) &&
+	q_to_nul >tag-NUL-header <<-EOF &&
+	object $sha
+	type commit
+	tag contains-Q-in-header
+	tagger T A Gger <[email protected]> 1234567890 -0000
+
+	This is an invalid tag.
+	EOF
+
+	tag=$(git hash-object --literally -t tag -w --stdin <tag-NUL-header) &&
+	test_when_finished "remove_object $tag" &&
+	echo $tag >.git/refs/tags/wrong &&
+	test_when_finished "git update-ref -d refs/tags/wrong" &&
+	test_must_fail git fsck --tags 2>out &&
+	cat out &&
+	grep "error in tag $tag.*unterminated header: NUL at offset" out
+'
+
 test_expect_success 'cleaned up' '
 	git fsck >actual 2>&1 &&
 	test_cmp empty actual
Signature · 1 line
2.6.3
#1 lore raw

PATCH 2/2 fsck: treat a NUL in a tag header as an error

We check the return value of verify_header() for commits already, so do the same for tags as well.

---
 fsck.c          | 3 ++-
 t/t1450-fsck.sh | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
fsck.c +2 -1
index e41e753..4060f1f 100644
@@ -711,7 +711,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
 		}
 	}
 
-	if (verify_headers(buffer, size, &tag->object, options))
+	ret = verify_headers(buffer, size, &tag->object, options);
+	if (ret)
 		goto done;
 
 	if (!skip_prefix(buffer, "object ", &buffer)) {
t/t1450-fsck.sh +1 -1
index 6c96953..e66b7cb 100755
@@ -288,7 +288,7 @@ test_expect_success 'tag with bad tagger' '
 	grep "error in tag .*: invalid author/committer" out
 '
 
-test_expect_failure 'tag with NUL in header' '
+test_expect_success 'tag with NUL in header' '
 	sha=$(git rev-parse HEAD) &&
 	q_to_nul >tag-NUL-header <<-EOF &&
 	object $sha
Signature · 1 line
2.6.3
#2 lore raw

Re: [PATCH 2/2] fsck: treat a NUL in a tag header as an error

Quoted 14 lines · René Scharfe

We check the return value of verify_header() for commits already, so do the same for tags as well.

---
 fsck.c          | 3 ++-
 t/t1450-fsck.sh | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
fsck.c +0 -0
index e41e753..4060f1f 100644

fsck.c static int fsck_tag_buffer(struct tag *tag, const char *data, · line 716

Quoted 7 lines
fsck.c +2 -1
@@ -711,7 +711,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
 		}
 	}
 
-	if (verify_headers(buffer, size, &tag->object, options))
+	ret = verify_headers(buffer, size, &tag->object, options);
+	if (ret)
 		goto done;

Good catch. The patch look reasonable to me.

-Peff

#3 lore raw

Re: [PATCH 2/2] fsck: treat a NUL in a tag header as an error

In reply to René Scharfe

Hi René,

Quoted 2 lines · René Scharfe

We check the return value of verify_header() for commits already, so do the same for tags as well.

Thanks for catching and fixing my bug!

Ciao,
Dscho

#4 lore raw
In reply to René Scharfe

t/t1450-fsck.sh test_expect_success 'integer overflow in timestamps is reported' ' · line 184

Quoted 13 lines · René Scharfe

---

t/t1450-fsck.sh +9 -0
@@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' '
+test_expect_success 'commit with NUL in header' '
+       git cat-file commit HEAD >basis &&
+       sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
+       new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
+       test_when_finished "remove_object $new" &&
+       git update-ref refs/heads/bogus "$new" &&
+       test_when_finished "git update-ref -d refs/heads/bogus" &&
+       test_must_fail git fsck 2>out &&
+       cat out &&

What is the purpose of this 'cat'?

Quoted 10 lines
t/t1450-fsck.sh +9 -0
+       grep "error in commit $new.*unterminated header: NUL at offset" out
+'
@@ -276,6 +288,26 @@ test_expect_success 'tag with bad tagger' '
+test_expect_failure 'tag with NUL in header' '
+       sha=$(git rev-parse HEAD) &&
+       q_to_nul >tag-NUL-header <<-EOF &&
+       object $sha
+       type commit
+       tag contains-Q-in-header
+       tagger T A Gger <[email protected]> 1234567890 -0000

t/t1450-fsck.sh test_expect_success 'tag with bad tagger' ' · line 304

Quoted 10 lines
t/t1450-fsck.sh +10 -0
+
+       This is an invalid tag.
+       EOF
+
+       tag=$(git hash-object --literally -t tag -w --stdin <tag-NUL-header) &&
+       test_when_finished "remove_object $tag" &&
+       echo $tag >.git/refs/tags/wrong &&
+       test_when_finished "git update-ref -d refs/tags/wrong" &&
+       test_must_fail git fsck --tags 2>out &&
+       cat out &&

Ditto.

Quoted 5 lines
t/t1450-fsck.sh +3 -1
+       grep "error in tag $tag.*unterminated header: NUL at offset" out
+'
+
--

2.6.3

#5 lore raw

Am 19.11.2015 um 21:33 schrieb Eric Sunshine:

Quoted 8 lines · René Scharfe
Quoted 7 lines

---

t/t1450-fsck.sh +3 -0
@@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' '
+test_expect_success 'commit with NUL in header' '
+       git cat-file commit HEAD >basis &&
+       sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
Quoted 8 lines

t/t1450-fsck.sh test_expect_success 'integer overflow in timestamps is reported' ' · line 187

Quoted 6 lines
t/t1450-fsck.sh +6 -0
+       new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
+       test_when_finished "remove_object $new" &&
+       git update-ref refs/heads/bogus "$new" &&
+       test_when_finished "git update-ref -d refs/heads/bogus" &&
+       test_must_fail git fsck 2>out &&
+       cat out &&

What is the purpose of this 'cat'?

It shows the full error message when the test is run with --debug, which is convenient when the following grep doesn't match. The same is done in most tests in that file.

Quoted 29 lines
Quoted 11 lines
t/t1450-fsck.sh +9 -0
+       grep "error in commit $new.*unterminated header: NUL at offset" out
+'
@@ -276,6 +288,26 @@ test_expect_success 'tag with bad tagger' '
+test_expect_failure 'tag with NUL in header' '
+       sha=$(git rev-parse HEAD) &&
+       q_to_nul >tag-NUL-header <<-EOF &&
+       object $sha
+       type commit
+       tag contains-Q-in-header
+       tagger T A Gger <[email protected]> 1234567890 -0000

t/t1450-fsck.sh test_expect_success 'tag with bad tagger' ' · line 304

Quoted 10 lines
t/t1450-fsck.sh +10 -0
+
+       This is an invalid tag.
+       EOF
+
+       tag=$(git hash-object --literally -t tag -w --stdin <tag-NUL-header) &&
+       test_when_finished "remove_object $tag" &&
+       echo $tag >.git/refs/tags/wrong &&
+       test_when_finished "git update-ref -d refs/tags/wrong" &&
+       test_must_fail git fsck --tags 2>out &&
+       cat out &&

Ditto.

Quoted 5 lines
t/t1450-fsck.sh +3 -1
+       grep "error in tag $tag.*unterminated header: NUL at offset" out
+'
+
--

2.6.3

#6 lore raw
Quoted 14 lines · René Scharfe
Quoted 5 lines
Quoted 5 lines
t/t1450-fsck.sh +3 -0
@@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' '
+test_expect_success 'commit with NUL in header' '
+       git cat-file commit HEAD >basis &&
+       sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
Quoted 8 lines

t/t1450-fsck.sh test_expect_success 'integer overflow in timestamps is reported' ' · line 187

Quoted 6 lines
t/t1450-fsck.sh +6 -0
+       new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
+       test_when_finished "remove_object $new" &&
+       git update-ref refs/heads/bogus "$new" &&
+       test_when_finished "git update-ref -d refs/heads/bogus" &&
+       test_must_fail git fsck 2>out &&
+       cat out &&

What is the purpose of this 'cat'?

Quoted 3 lines

It shows the full error message when the test is run with --debug, which is convenient when the following grep doesn't match. The same is done in most tests in that file.

I'm slightly negative on such a construct, just because it wastes a process in the case where we are not in --verbose mode. I don't mind it in this patch in the spirit of consistency within t1450, but I think we should probably avoid spreading it.

-Peff

#7 lore raw

Am 20.11.2015 um 12:14 schrieb Jeff King:

Quoted 16 lines · René Scharfe
Quoted 15 lines
Quoted 14 lines

t/t1450-fsck.sh test_expect_success 'integer overflow in timestamps is reported' ' · line 184

Quoted 12 lines
t/t1450-fsck.sh +9 -0
@@ -176,6 +176,18 @@ test_expect_success 'integer overflow in timestamps is reported' '
+test_expect_success 'commit with NUL in header' '
+       git cat-file commit HEAD >basis &&
+       sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
+       new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
+       test_when_finished "remove_object $new" &&
+       git update-ref refs/heads/bogus "$new" &&
+       test_when_finished "git update-ref -d refs/heads/bogus" &&
+       test_must_fail git fsck 2>out &&
+       cat out &&

What is the purpose of this 'cat'?

Quoted 8 lines
Quoted 3 lines

It shows the full error message when the test is run with --debug, which is convenient when the following grep doesn't match. The same is done in most tests in that file.

I'm slightly negative on such a construct, just because it wastes a process in the case where we are not in --verbose mode. I don't mind it in this patch in the spirit of consistency within t1450, but I think we should probably avoid spreading it.

This practice is not used that much (yet). We can contain it by providing an alternative. I'll send patches for adding a helper function similar to test_must_be_empty for that.

René

#8 lore raw

[PATCH 3/2] test: factor out helper function test_must_contain

In reply to Jeff King

Extract a helper function for searching for a pattern in a file and printing the whole file if the pattern is not found. It is useful when starting tests with --verbose for debugging purposes.

---
 t/t0008-ignores.sh      | 10 +---------
 t/test-lib-functions.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 9 deletions(-)
t/t0008-ignores.sh +1 -9
index 4ef5ed4..0414c01 100755
@@ -46,15 +46,7 @@ broken_c_unquote_verbose () {
 }
 
 stderr_contains () {
-	regexp="$1"
-	if grep "$regexp" "$HOME/stderr"
-	then
-		return 0
-	else
-		echo "didn't find /$regexp/ in $HOME/stderr"
-		cat "$HOME/stderr"
-		return 1
-	fi
+	test_must_contain "$1" "$HOME/stderr"
 }
 
 stderr_empty_on_success () {
t/test-lib-functions.sh +12 -0
index 73e37a1..eb44cb4 100644
@@ -689,6 +689,18 @@ test_must_be_empty () {
 	fi
 }
 
+# Check if a file contains an expected pattern.
+test_must_contain () {
+	if grep "$1" "$2"
+	then
+		return 0
+	else
+		echo "didn't find /$1/ in '$2', it contains:"
+		cat "$2"
+		return 1
+	fi
+}
+
 # Tests that its two parameters refer to the same revision
 test_cmp_rev () {
 	git rev-parse --verify "$1" >expect.rev &&
Signature · 1 line
2.6.3
#9 lore raw

Re: [PATCH 3/2] test: factor out helper function test_must_contain

Am 20.11.2015 um 21:50 schrieb René Scharfe:

Quoted 5 lines

Extract a helper function for searching for a pattern in a file and printing the whole file if the pattern is not found. It is useful when starting tests with --verbose for debugging purposes.

t/test-lib-functions.sh +1 -0
+# Check if a file contains an expected pattern.

t/test-lib-functions.sh test_must_be_empty () { · line 702

Quoted 10 lines
t/test-lib-functions.sh +10 -0
+test_must_contain () {
+	if grep "$1" "$2"
+	then
+		return 0
+	else
+		echo "didn't find /$1/ in '$2', it contains:"
+		cat "$2"
+		return 1
+	fi
+}

There is already test_i18n_grep. Should it be folded into this function? Wouldn't we also want to have a function test_must_not_contain?

IMHO, we should not increase the number of functions that give a bonus only when there is a test case failure. They do not scale well: There is a permanent mental burden on every reviewer to watch out that they are used in new tests. But without those functions, the burden is on the one person investigating a test case failure, who has to live without the debugging support.

-- Hannes

#10 lore raw

Re: [PATCH 3/2] test: factor out helper function test_must_contain

Am 21.11.2015 um 09:11 schrieb Johannes Sixt:

Quoted 5 lines

Am 20.11.2015 um 21:50 schrieb René Scharfe:

Quoted 3 lines

Extract a helper function for searching for a pattern in a file and printing the whole file if the pattern is not found. It is useful when starting tests with --verbose for debugging purposes.

Quoted 13 lines

Review

Quoted 11 lines
(quoted diff) +11 -0
+# Check if a file contains an expected pattern.
+test_must_contain () {
+    if grep "$1" "$2"
+    then
+        return 0
+    else
+        echo "didn't find /$1/ in '$2', it contains:"
+        cat "$2"
+        return 1
+    fi
+}

There is already test_i18n_grep. Should it be folded into this function?

That's a good point. But how? test_i18ngrep can also work as a filter and pass on options, so we'd need to parse all parameters and redirect stdin to a temporary file unless a file was specified. Or we could be sloppy and just check if the last parameter is a file and if yes then spew it out.

Quoted 1 line

Wouldn't we also want to have a function test_must_not_contain?

I doubt it. In such a function grep would display the lines that match unexpectedly already, so showing the whole file after that won't add much more of interest.

Quoted 6 lines

IMHO, we should not increase the number of functions that give a bonus only when there is a test case failure. They do not scale well: There is a permanent mental burden on every reviewer to watch out that they are used in new tests. But without those functions, the burden is on the one person investigating a test case failure, who has to live without the debugging support.

test_must_contain doesn't have to be used everywhere, only in cases where a file is shown and grepped. I agree that letting an existing function do that job (or deciding that the job is not worth doing) is preferable.

Here's how I imagine the sloppy add-on to test_i18ncmp to look:

t/test-lib.sh +15 -0
index 16c4d7b..db64600 100644
@@ -985,13 +985,28 @@ test_i18ncmp () {
 test_i18ngrep () {
 	if test -n "$GETTEXT_POISON"
 	then
 	    : # pretend success
 	elif test "x!" = "x$1"
 	then
 		shift
 		! grep "$@"
 	else
 		grep "$@"
+
+		rc=$?
+		if test $rc != 0
+		then
+			while test $# -gt 1
+			do
+				shift
+			done
+			if test -f "$1"
+			then
+				echo "Expected pattern not found, content is:"
+				cat "$1"
+			fi
+			return $rc
+		fi
 	fi
 }
 
#11 lore raw

[PATCH 4/2] test: use test_must_contain

In reply to Jeff King

Some tests print a file using cat and then grep it for some pattern. This is done to aid debugging in case the pattern is unexpectedly not found; the file contents can easily inspected by running the test with the option --verbose.

Replace these combinations by calls of test_must_contain. It only shows the whole file if the pattern is not found, saving a cat call (and thus a fork(2)) in the normal case. It also just needs a single line and makes the intent clearer.

The flag -q in t5801 is dropped as there's no pressing reason for hiding the matching line in this test. Consistency is more important, and none of the remaining grep calls in that file use that flag or redirect the results to /dev/null.

---
 t/t1450-fsck.sh           | 52 ++++++++++++++++-------------------------------
 t/t5510-fetch.sh          |  3 +--
 t/t5801-remote-helpers.sh |  3 +--
 t/t9810-git-p4-rcs.sh     |  3 +--
 4 files changed, 21 insertions(+), 40 deletions(-)
t/t1450-fsck.sh +18 -34
index e66b7cb..190c97f 100755
@@ -70,16 +70,14 @@ test_expect_success 'object with bad sha1' '
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "$sha.*corrupt" out
+	test_must_contain "$sha.*corrupt" out
 '
 
 test_expect_success 'branch pointing to non-commit' '
 	git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
 	test_when_finished "git update-ref -d refs/heads/invalid" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "not a commit" out
+	test_must_contain "not a commit" out
 '
 
 test_expect_success 'HEAD link pointing at a funny object' '
@@ -88,8 +86,7 @@ test_expect_success 'HEAD link pointing at a funny object' '
 	echo 0000000000000000000000000000000000000000 >.git/HEAD &&
 	# avoid corrupt/broken HEAD from interfering with repo discovery
 	test_must_fail env GIT_DIR=.git git fsck 2>out &&
-	cat out &&
-	grep "detached HEAD points" out
+	test_must_contain "detached HEAD points" out
 '
 
 test_expect_success 'HEAD link pointing at a funny place' '
@@ -98,8 +95,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
 	echo "ref: refs/funny/place" >.git/HEAD &&
 	# avoid corrupt/broken HEAD from interfering with repo discovery
 	test_must_fail env GIT_DIR=.git git fsck 2>out &&
-	cat out &&
-	grep "HEAD points to something strange" out
+	test_must_contain "HEAD points to something strange" out
 '
 
 test_expect_success 'email without @ is okay' '
@@ -122,8 +118,7 @@ test_expect_success 'email with embedded > is not okay' '
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "error in commit $new" out
+	test_must_contain "error in commit $new" out
 '
 
 test_expect_success 'missing < email delimiter is reported nicely' '
@@ -134,8 +129,7 @@ test_expect_success 'missing < email delimiter is reported nicely' '
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "error in commit $new.* - bad name" out
+	test_must_contain "error in commit $new.* - bad name" out
 '
 
 test_expect_success 'missing email is reported nicely' '
@@ -146,8 +140,7 @@ test_expect_success 'missing email is reported nicely' '
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "error in commit $new.* - missing email" out
+	test_must_contain "error in commit $new.* - missing email" out
 '
 
 test_expect_success '> in name is reported' '
@@ -158,8 +151,7 @@ test_expect_success '> in name is reported' '
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "error in commit $new" out
+	test_must_contain "error in commit $new" out
 '
 
 # date is 2^64 + 1
@@ -172,8 +164,7 @@ test_expect_success 'integer overflow in timestamps is reported' '
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "error in commit $new.*integer overflow" out
+	test_must_contain "error in commit $new.*integer overflow" out
 '
 
 test_expect_success 'commit with NUL in header' '
@@ -184,8 +175,7 @@ test_expect_success 'commit with NUL in header' '
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
-	grep "error in commit $new.*unterminated header: NUL at offset" out
+	test_must_contain "error in commit $new.*unterminated header: NUL at offset" out
 '
 
 test_expect_success 'malformatted tree object' '
@@ -205,7 +195,7 @@ test_expect_success 'malformatted tree object' '
 		git hash-object -w -t tree --stdin
 	) &&
 	test_must_fail git fsck 2>out &&
-	grep "error in tree .*contains duplicate file entries" out
+	test_must_contain "error in tree .*contains duplicate file entries" out
 '
 
 test_expect_success 'tag pointing to nonexistent' '
@@ -223,8 +213,7 @@ test_expect_success 'tag pointing to nonexistent' '
 	echo $tag >.git/refs/tags/invalid &&
 	test_when_finished "git update-ref -d refs/tags/invalid" &&
 	test_must_fail git fsck --tags >out &&
-	cat out &&
-	grep "broken link" out
+	test_must_contain "broken link" out
 '
 
 test_expect_success 'tag pointing to something else than its type' '
@@ -285,7 +274,7 @@ test_expect_success 'tag with bad tagger' '
 	echo $tag >.git/refs/tags/wrong &&
 	test_when_finished "git update-ref -d refs/tags/wrong" &&
 	test_must_fail git fsck --tags 2>out &&
-	grep "error in tag .*: invalid author/committer" out
+	test_must_contain "error in tag .*: invalid author/committer" out
 '
 
 test_expect_success 'tag with NUL in header' '
@@ -304,8 +293,7 @@ test_expect_success 'tag with NUL in header' '
 	echo $tag >.git/refs/tags/wrong &&
 	test_when_finished "git update-ref -d refs/tags/wrong" &&
 	test_must_fail git fsck --tags 2>out &&
-	cat out &&
-	grep "error in tag $tag.*unterminated header: NUL at offset" out
+	test_must_contain "error in tag $tag.*unterminated header: NUL at offset" out
 '
 
 test_expect_success 'cleaned up' '
@@ -335,8 +323,7 @@ test_expect_success 'rev-list --verify-objects with bad sha1' '
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 
 	test_might_fail git rev-list --verify-objects refs/heads/bogus >/dev/null 2>out &&
-	cat out &&
-	grep -q "error: sha1 mismatch 63ffffffffffffffffffffffffffffffffffffff" out
+	test_must_contain "error: sha1 mismatch 63ffffffffffffffffffffffffffffffffffffff" out
 '
 
 test_expect_success 'force fsck to ignore double author' '
@@ -360,8 +347,7 @@ test_expect_success 'fsck notices blob entry pointing to null sha1' '
 	 sha=$(printf "100644 file$_bz$_bz20" |
 	       git hash-object -w --stdin -t tree) &&
 	  git fsck 2>out &&
-	  cat out &&
-	  grep "warning.*null sha1" out
+	  test_must_contain "warning.*null sha1" out
 	)
 '
 
@@ -371,8 +357,7 @@ test_expect_success 'fsck notices submodule entry pointing to null sha1' '
 	 sha=$(printf "160000 submodule$_bz$_bz20" |
 	       git hash-object -w --stdin -t tree) &&
 	  git fsck 2>out &&
-	  cat out &&
-	  grep "warning.*null sha1" out
+	  test_must_contain "warning.*null sha1" out
 	)
 '
 
@@ -392,8 +377,7 @@ while read name path pretty; do
 			printf "$mode $type %s\t%s" "$value" "$path" >bad &&
 			bad_tree=$(git mktree <bad) &&
 			git fsck 2>out &&
-			cat out &&
-			grep "warning.*tree $bad_tree" out
+			test_must_contain "warning.*tree $bad_tree" out
 		)'
 	done <<-\EOF
 	100644 blob
t/t5510-fetch.sh +1 -2
index 0ba9db0..f5023b0 100755
@@ -393,8 +393,7 @@ test_expect_success 'fetch from GIT URL with a non-applying branch.<name>.merge
 # the strange name is: a\!'b
 test_expect_success 'quoting of a strangely named repo' '
 	test_must_fail git fetch "a\\!'\''b" > result 2>&1 &&
-	cat result &&
-	grep "fatal: '\''a\\\\!'\''b'\''" result
+	test_must_contain "fatal: '\''a\\\\!'\''b'\''" result
 '
 
 test_expect_success 'bundle should record HEAD correctly' '
t/t5801-remote-helpers.sh +1 -2
index 362b158..57ec852 100755
@@ -245,8 +245,7 @@ clean_mark () {
 test_expect_success 'proper failure checks for fetching' '
 	(cd local &&
 	test_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git fetch 2>error &&
-	cat error &&
-	grep -q "Error while running fast-import" error
+	test_must_contain "Error while running fast-import" error
 	)
 '
 
t/t9810-git-p4-rcs.sh +1 -2
index 8134ab4..e02b490 100755
@@ -294,8 +294,7 @@ test_expect_success 'cope with rcs keyword file deletion' '
 		echo "\$Revision\$" >kwdelfile.c &&
 		p4 add -t ktext kwdelfile.c &&
 		p4 submit -d "Add file to be deleted" &&
-		cat kwdelfile.c &&
-		grep 1 kwdelfile.c
+		test_must_constain 1 kwdelfile.c
 	) &&
 	git p4 clone --dest="$git" //depot &&
 	(
Signature · 1 line
2.6.3
#12 lore raw

Re: [PATCH 4/2] test: use test_must_contain

Hi,

t/t9810-git-p4-rcs.sh test_expect_success 'cope with rcs keyword file deletion' ' · line 297

Quoted 11 lines
t/t9810-git-p4-rcs.sh +1 -2
index 8134ab4..e02b490 100755
@@ -294,8 +294,7 @@ test_expect_success 'cope with rcs keyword file deletion' '
 		echo "\$Revision\$" >kwdelfile.c &&
 		p4 add -t ktext kwdelfile.c &&
 		p4 submit -d "Add file to be deleted" &&
-		cat kwdelfile.c &&
-		grep 1 kwdelfile.c
+		test_must_constain 1 kwdelfile.c

s/constain/contain/

#13 lore raw

Re: [PATCH 4/2] test: use test_must_contain

Am 21.11.2015 um 02:16 schrieb SZEDER Gábor:

Quoted 16 lines

Hi,

t/t9810-git-p4-rcs.sh test_expect_success 'cope with rcs keyword file deletion' ' · line 297

Quoted 11 lines
t/t9810-git-p4-rcs.sh +1 -2
index 8134ab4..e02b490 100755
@@ -294,8 +294,7 @@ test_expect_success 'cope with rcs keyword file deletion' '
  		echo "\$Revision\$" >kwdelfile.c &&
  		p4 add -t ktext kwdelfile.c &&
  		p4 submit -d "Add file to be deleted" &&
-		cat kwdelfile.c &&
-		grep 1 kwdelfile.c
+		test_must_constain 1 kwdelfile.c

s/constain/contain/

Thank you! I need to work on my vi fu.. Or install all optional tools to be able to run the full test suite. ;)

René