git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH] tests: clean after SANITY tests
Date: Fri, 15 Jun 2018 11:30:48 -0700	[thread overview]
Message-ID: <xmqqa7rv297r.fsf@gitster-ct.c.googlers.com> (raw)

Some of our tests try to make sure Git behaves sensibly in a
read-only directory, by dropping 'w' permission bit before doing a
test and then restoring it after it is done.  The latter is needed
for the test framework to clean after itself without leaving a
leftover directory that cannot be removed.

Ancient parts of tests however arrange the above with

	chmod a-w . &&
	... do the test ...
	status=$?
	chmod 775 .
	(exit $status)

which obviously would not work if the test somehow dies before it
has the chance to do "chmod 775".  Rewrite them by following a more
robust pattern recently written tests use, which is

	test_when_finished "chmod 775 ." &&
	chmod a-w . &&
	... do the test ...

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I happened to hit ^C at the right moment and saw "make distclean"
   fail miserably.  I guess you can still kill the process in such a
   way that when-finished hander would not fire, so in that sense
   this is more about consistency and uniformity than the actual
   safety, but anyway...

 t/t0001-init.sh             | 1 +
 t/t0070-fundamental.sh      | 2 +-
 t/t1004-read-tree-m-u-wf.sh | 6 ++----
 t/t5537-fetch-shallow.sh    | 2 +-
 t/t7508-status.sh           | 4 +---
 5 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index c413bff9cf..4c865051e7 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -287,6 +287,7 @@ test_expect_success 'init notices EEXIST (2)' '
 '
 
 test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
+	test_when_finished "chmod +w newdir" &&
 	rm -fr newdir &&
 	mkdir newdir &&
 	chmod -w newdir &&
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 991ed2a48d..60e3de7b7b 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -19,8 +19,8 @@ test_expect_success 'mktemp to nonexistent directory prints filename' '
 
 test_expect_success POSIXPERM,SANITY 'mktemp to unwritable directory prints filename' '
 	mkdir cannotwrite &&
-	chmod -w cannotwrite &&
 	test_when_finished "chmod +w cannotwrite" &&
+	chmod -w cannotwrite &&
 	test_must_fail test-mktemp cannotwrite/testXXXXXX 2>err &&
 	grep "cannotwrite/test" err
 '
diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
index c7ce5d8bb5..826cd32e23 100755
--- a/t/t1004-read-tree-m-u-wf.sh
+++ b/t/t1004-read-tree-m-u-wf.sh
@@ -179,6 +179,8 @@ test_expect_success 'funny symlink in work tree' '
 
 test_expect_success SANITY 'funny symlink in work tree, un-unlink-able' '
 
+	test_when_finished "chmod a+w a 2>/dev/null; rm -fr a b" &&
+
 	rm -fr a b &&
 	git reset --hard &&
 
@@ -188,10 +190,6 @@ test_expect_success SANITY 'funny symlink in work tree, un-unlink-able' '
 
 '
 
-# clean-up from the above test
-chmod a+w a 2>/dev/null
-rm -fr a b
-
 test_expect_success 'D/F setup' '
 
 	git reset --hard &&
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index df8d2f095a..943231af96 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -175,8 +175,8 @@ EOF
 
 test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
 	cp -R .git read-only.git &&
-	find read-only.git -print | xargs chmod -w &&
 	test_when_finished "find read-only.git -type d -print | xargs chmod +w" &&
+	find read-only.git -print | xargs chmod -w &&
 	git clone --no-local --depth=2 read-only.git from-read-only &&
 	git --git-dir=from-read-only/.git log --format=%s >actual &&
 	cat >expect <<EOF &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 50052e2872..10b084d890 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1099,6 +1099,7 @@ EOF
 '
 
 test_expect_success POSIXPERM,SANITY 'status succeeds in a read-only repository' '
+	test_when_finished "chmod 775 .git" &&
 	(
 		chmod a-w .git &&
 		# make dir1/tracked stat-dirty
@@ -1108,9 +1109,6 @@ test_expect_success POSIXPERM,SANITY 'status succeeds in a read-only repository'
 		# make sure "status" succeeded without writing index out
 		git diff-files | grep dir1/tracked
 	)
-	status=$?
-	chmod 775 .git
-	(exit $status)
 '
 
 (cd sm && echo > bar && git add bar && git commit -q -m 'Add bar') && git add sm
-- 
2.18.0-rc2


             reply	other threads:[~2018-06-15 18:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-15 18:30 Junio C Hamano [this message]
2018-06-15 19:57 ` [PATCH] tests: clean after SANITY tests Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa7rv297r.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).