threads / patch / 34626

patchgit_mkstemps: add test suite test

Subject: [PATCH revised] git_mkstemps: add test suite test

## tl;dr

5 messages between Aug 6, 2013 and Aug 6, 2013. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Dale R. Worley· Aug 6, 2013, 18:05 UTC · lore

Commit a2cb86 ("git_mkstemps: correctly test return value of open()", 12 Jul 2013) fixes a bug regarding testing the return of an open() call for success/failure. Add a testsuite test for that fix. The test exercises a situation where that open() is known to return 0.

Signed-off-by: Dale Worley <worley@ariadne.com>
---
This version of the patch cleans up a number of errors in my previous
version (which were ultimately due to my faulty updating of my master
branch).  The commit that added the open() test is now correctly
described.  Since the test was not present in the test suite at all,
the patch is described as adding the test rather than improving it.

a2cb86 is on branch tr/fd-gotcha-fixes, but that has been merged into master now.

(Thanks for your patience with this.)
Dale
 t/t0070-fundamental.sh | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/t0070-fundamental.sh +7 −0
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 986b2a8..d427f3a 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,6 +25,13 @@ test_expect_success POSIXPERM,SANITY 'mktemp to unwritable directory prints file
 	grep "cannotwrite/test" err
 '
 
+test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' '
+	git init &&
+	echo Test. >test-file &&
+	git add test-file &&
+	git commit -m Message. <&-
+'
+
 test_expect_success 'check for a bug in the regex routines' '
 	# if this test fails, re-build git with NO_REGEX=1
 	test-regex

-- 
1.8.4.rc1.24.gd407a5c
Junio C Hamano· Aug 6, 2013, 18:17 UTC · re: Dale R. Worley · lore

Re: [PATCH revised] git_mkstemps: add test suite test

worley@alum.mit.edu (Dale R. Worley) writes:
Show 15 quoted lines
> Commit a2cb86 ("git_mkstemps: correctly test return value of open()",
> 12 Jul 2013) fixes a bug regarding testing the return of an open()
> call for success/failure.  Add a testsuite test for that fix.  The
> test exercises a situation where that open() is known to return 0.
>
> Signed-off-by: Dale Worley <worley@ariadne.com>
> ---
> This version of the patch cleans up a number of errors in my previous
> version (which were ultimately due to my faulty updating of my master
> branch).  The commit that added the open() test is now correctly
> described.  Since the test was not present in the test suite at all,
> the patch is described as adding the test rather than improving it.
>
> a2cb86 is on branch tr/fd-gotcha-fixes, but that has been merged into
> master now.
Thanks. I thought I've already queued 
Message-ID: <7vfvuokpr0.fsf@alter.siamese.dyndns.org>
aka 
http://article.gmane.org/gmane.comp.version-control.git/231680
which tests
    git commit --allow-empty -m message <&-
> +test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' '
> +	git init &&

This does not do anything useful; you are in the test playpen aka "trash" which is an already initialized git repository.

> +	echo Test. >test-file &&
> +	git add test-file &&
You do not have to have extra contents...
> +	git commit -m Message. <&-
...you can do with just "--allow-empty" instead.
Dale R. Worley· Aug 6, 2013, 18:59 UTC · re: Junio C Hamano · lore

Re: [PATCH revised] git_mkstemps: add test suite test

Show 11 quoted lines
> From: Junio C Hamano <gitster@pobox.com>
> 
> Thanks. I thought I've already queued 
> 
> Message-ID: <7vfvuokpr0.fsf@alter.siamese.dyndns.org>
> aka 
> http://article.gmane.org/gmane.comp.version-control.git/231680
> 
> which tests
> 
>     git commit --allow-empty -m message <&-

My mistake... I've been so intent on revising my repository and rewriting the patch that I overlooked that you'd done the revision already.

Dale
Dale R. Worley· Aug 6, 2013, 19:02 UTC · re: Junio C Hamano · lore

Re: [PATCH revised] git_mkstemps: add test suite test

>     git commit --allow-empty -m message <&-

Though as of [fb56570] "Sync with maint to grab trivial doc fixes", that test doesn't fail for me if I revert to

		fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
		if (fd > 0)
			return fd;

I haven't been watching the code changes carefully; has there been a fix that is expected to cause that?

Dale
Junio C Hamano· Aug 6, 2013, 20:50 UTC · re: Dale R. Worley · lore

Re: [PATCH revised] git_mkstemps: add test suite test

worley@alum.mit.edu (Dale R. Worley) writes:
Show 13 quoted lines
>>     git commit --allow-empty -m message <&-
>
> Though as of [fb56570] "Sync with maint to grab trivial doc fixes",
> that test doesn't fail for me if I revert to
>
> 		fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
> 		if (fd > 0)
> 			return fd;
>
> I haven't been watching the code changes carefully; has there been a
> fix that is expected to cause that?
>
> Dale

That is because a11c3964 (git: ensure 0/1/2 are open in main(), 2013-07-16) happened in the meantime, I think.

← back to recent threads