threads / patch / 34594

patchgit_mkstemps: improve test suite test

Subject: [PATCH] git_mkstemps: improve test suite test

## tl;dr

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

replies: 3people: 3as markdown or json

Dale R. Worley· Aug 3, 2013, 00:27 UTC · lore

Commit 52749 fixes a bug regarding testing the return of an open() call for success/failure. Improve the testsuite test for that fix by removing the helper program 'test-close-fd-0' and replacing it with the shell redirection '<&-'. (The redirection is Posix, so it should be portable.)

Signed-off-by: Dale Worley <worley@ariadne.com>
---
Show 5 quoted lines
> From: Junio C Hamano <gitster@pobox.com>
> Date: Fri, 19 Jul 2013 07:29:47 -0700
> 
> The change itself looks good; care to write it up as a proper patch
> with a proposed log message?

My apologies for the delay; I've had to do some yak-shaving to learn how to construct patches properly. (I've written some clarifications for Document/SubmittingPatches, which I will submit separately.)

Someone has gone ahead and made the code change, so all that remains is to update the testsuite test by replacing the helper program 'test-close-fd-0' with the Posix shell redirection '<&-'.

Dale
 Makefile          |    1 -
 test-close-fd-0.c |   14 --------------
 2 files changed, 0 insertions(+), 15 deletions(-)
 delete mode 100644 test-close-fd-0.c
Show changes to 2 files +0 −15

Makefile, test-close-fd-0.c

diff --git a/Makefile b/Makefile
index 8ad40d4..3588ca1 100644
--- a/Makefile
+++ b/Makefile
@@ -557,7 +557,6 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
-TEST_PROGRAMS_NEED_X += test-close-fd-0
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
diff --git a/test-close-fd-0.c b/test-close-fd-0.c
deleted file mode 100644
index 3745c34..0000000
--- a/test-close-fd-0.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include <unistd.h>
-
-/* Close file descriptor 0 (which is standard-input), then execute the
- * remainder of the command line as a command. */
-
-int main(int argc, char **argv)
-{
-	/* Close fd 0. */
-	close(0);
-	/* Execute the requested command. */
-	execvp(argv[1], &argv[1]);
-	/* If execve() failed, return an error. */
-	return 1;
-}
-- 
1.7.7.6
Jonathan Nieder· Aug 3, 2013, 00:36 UTC · re: Dale R. Worley · lore

Re: [PATCH] git_mkstemps: improve test suite test

Hi,
Dale R. Worley wrote:
> Commit 52749 fixes a bug regarding testing the return of an open()
 $ git show 52749
 fatal: ambiguous argument '52749': unknown revision or path not in the working tree.
Could you mention its subject line or date so it's easier to find?
Show 6 quoted lines
> call for success/failure.  Improve the testsuite test for that fix by
> removing the helper program 'test-close-fd-0' and replacing it with
> the shell redirection '<&-'.  (The redirection is Posix, so it should
> be portable.)
> 
> Signed-off-by: Dale Worley <worley@ariadne.com>
[...]
> Someone has gone ahead and made the code change, so all that remains
> is to update the testsuite test by replacing the helper program
> 'test-close-fd-0' with the Posix shell redirection '<&-'.

The above paragraph should be part of the commit message, since otherwise the patch is hard to understand.

The patch text looks good.

Thanks and hope that helps, Jonathan

Junio C Hamano· Aug 5, 2013, 16:21 UTC · re: Dale R. Worley · lore

Re: [PATCH] git_mkstemps: improve test suite test

worley@alum.mit.edu (Dale R. Worley) writes:
Show 8 quoted lines
> Commit 52749 fixes a bug regarding testing the return of an open()
> call for success/failure.  Improve the testsuite test for that fix by
> removing the helper program 'test-close-fd-0' and replacing it with
> the shell redirection '<&-'.  (The redirection is Posix, so it should
> be portable.)
>
> Signed-off-by: Dale Worley <worley@ariadne.com>
> ---

Sorry, but I have no idea what commit you are talking about, and as far as I can see there is no such file test-close-fd-0.c in my tree.

Puzzled...
>  Makefile          |    1 -
>  test-close-fd-0.c |   14 --------------
>  2 files changed, 0 insertions(+), 15 deletions(-)
>  delete mode 100644 test-close-fd-0.c
Junio C Hamano· Aug 5, 2013, 17:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] git_mkstemps: improve test suite test

Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
> worley@alum.mit.edu (Dale R. Worley) writes:
>
>> Commit 52749 fixes a bug regarding testing the return of an open()
>> call for success/failure.  Improve the testsuite test for that fix by
>> removing the helper program 'test-close-fd-0' and replacing it with
>> the shell redirection '<&-'.  (The redirection is Posix, so it should
>> be portable.)
>>
>> Signed-off-by: Dale Worley <worley@ariadne.com>
>> ---
>
> Sorry, but I have no idea what commit you are talking about, and as
> far as I can see there is no such file test-close-fd-0.c in my tree.
>
> Puzzled...

OK, let's do this on top of a77f106c (run-command: dup_devnull(): guard against syscalls failing, 2013-07-12) which is at the tip of tr/fd-gotcha-fixes that contains the earlier fix.

-- >8 --
From: "Dale R. Worley" <worley@alum.mit.edu>
Date: Fri, 2 Aug 2013 20:27:23 -0400
Subject: [PATCH] t0070: test that git_mkstemps correctly checks return value of open()
Signed-off-by: Dale R. Worley <worley@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0070-fundamental.sh | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to t/t0070-fundamental.sh +4 −0
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index da2c504..ff3776f 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,6 +25,10 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '
 	grep "cannotwrite/test" err
 '
 
+test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' '
+	git commit --allow-empty -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-129-g1f3472b

← back to recent threads