{"thread":{"id":"33509","subject":"[PATCH] t/test-lib.sh: fix TRASH_DIRECTORY handling","startedAt":"2013-04-14T16:34:56Z","lastAt":"2013-04-14T19:44:36Z","messageCount":3,"participants":["John Keeping","Jeff King","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"214217","messageId":"11cf45ad9779240b588da4b25c8ae5b6cc61b427.1365957201.git.john@keeping.me.uk","threadId":"33509","inReplyTo":null,"subject":"[PATCH] t/test-lib.sh: fix TRASH_DIRECTORY handling","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-04-14T16:34:56Z","receivedAt":"2013-04-14T16:34:56Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"After the location of $TRASH_DIRECTORY is adjusted by\n$TEST_OUTPUT_DIRECTORY, we go on to use the $test variable to make the\ntrash directory and cd into it. This means that when\n$TEST_OUTPUT_DIRECTORY is not \".\" and an absolute --root has not been\nspecified, we do not remove the trash directory once the tests are\ncomplete (remove_trash is set to $TRASH_DIRECTORY).\n\nFix this by always referring to the trash directory as $TRASH_DIRECTORY.\n\nSigned-off-by: John Keeping \n---\n t/test-lib.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex debd8b4..f79745c 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -607,7 +607,7 @@ case \"$test\" in\n *) TRASH_DIRECTORY=\"$TEST_OUTPUT_DIRECTORY/$test\" ;;\n esac\n test ! -z \"$debug\" || remove_trash=$TRASH_DIRECTORY\n-rm -fr \"$test\" || {\n+rm -fr \"$TRASH_DIRECTORY\" || {\n \tGIT_EXIT_OK=t\n \techo >&5 \"FATAL: Cannot prepare test area\"\n \texit 1\n@@ -618,13 +618,13 @@ export HOME\n \n if test -z \"$TEST_NO_CREATE_REPO\"\n then\n-\ttest_create_repo \"$test\"\n+\ttest_create_repo \"$TRASH_DIRECTORY\"\n else\n-\tmkdir -p \"$test\"\n+\tmkdir -p \"$TRASH_DIRECTORY\"\n fi\n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n-cd -P \"$test\" || exit 1\n+cd -P \"$TRASH_DIRECTORY\" || exit 1\n \n this_test=${0##*/}\n this_test=${this_test%%-*}\n-- \n1.8.2.694.ga76e9c3.dirty\n"},{"id":"214229","messageId":"20130414193821.GC4599@sigill.intra.peff.net","threadId":"33509","inReplyTo":"11cf45ad9779240b588da4b25c8ae5b6cc61b427.1365957201.git.john@keeping.me.uk","subject":"Re: [PATCH] t/test-lib.sh: fix TRASH_DIRECTORY handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-14T19:38:21Z","receivedAt":"2013-04-14T19:38:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 14, 2013 at 05:34:56PM +0100, John Keeping wrote:\n\n> After the location of $TRASH_DIRECTORY is adjusted by\n> $TEST_OUTPUT_DIRECTORY, we go on to use the $test variable to make the\n> trash directory and cd into it. This means that when\n> $TEST_OUTPUT_DIRECTORY is not \".\" and an absolute --root has not been\n> specified, we do not remove the trash directory once the tests are\n> complete (remove_trash is set to $TRASH_DIRECTORY).\n> \n> Fix this by always referring to the trash directory as $TRASH_DIRECTORY.\n\nThanks, this seems to date back all the way to my f423ef5 (tests: allow\nuser to specify trash directory location, 2009-08-09), although I think\nat that time it was not even possible to run the tests from any other\ndirectory. So I am happy to blame Thomas's later patches for violating\nmy assumptions. :)\n\nDefinitely:\n\n Acked-by: Jeff King \n\nIt probably makes sense to do the patch below on top, so that the error\ndoesn't happen again (I'd just as soon have it squashed into your patch,\nsince then then the motivation is even more obvious).\n\n-- >8 --\nSubject: [PATCH] t/test-lib.sh: drop \"$test\" variable\n\nThe $test variable is used as an interim buffer for\nconstructing $TRASH_DIRECTORY, and is almost compatible with\nit (the exception being that $test has not been converted to\nan absolute path). Let's get rid of it entirely so that\nlater code does not accidentally use it, thinking the two\nare interchangeable.\n\nSigned-off-by: Jeff King \n---\n t/test-lib.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a16bc73..b3f2488 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -600,11 +600,11 @@ fi\n fi\n \n # Test repository\n-test=\"trash directory.$(basename \"$0\" .sh)\"\n-test -n \"$root\" && test=\"$root/$test\"\n-case \"$test\" in\n-/*) TRASH_DIRECTORY=\"$test\" ;;\n- *) TRASH_DIRECTORY=\"$TEST_OUTPUT_DIRECTORY/$test\" ;;\n+TRASH_DIRECTORY=\"trash directory.$(basename \"$0\" .sh)\"\n+test -n \"$root\" && TRASH_DIRECTORY=\"$root/$TRASH_DIRECTORY\"\n+case \"$TRASH_DIRECTORY\" in\n+/*) ;; # absolute path is good\n+ *) TRASH_DIRECTORY=\"$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY\" ;;\n esac\n test ! -z \"$debug\" || remove_trash=$TRASH_DIRECTORY\n rm -fr \"$TRASH_DIRECTORY\" || {\n-- \n1.8.2.rc0.33.gd915649\n"},{"id":"214230","messageId":"87d2twzyl7.fsf@hexa.v.cablecom.net","threadId":"33509","inReplyTo":"20130414193821.GC4599@sigill.intra.peff.net","subject":"Re: [PATCH] t/test-lib.sh: fix TRASH_DIRECTORY handling","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-04-14T19:44:36Z","receivedAt":"2013-04-14T19:44:36Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King writes:\n\n> On Sun, Apr 14, 2013 at 05:34:56PM +0100, John Keeping wrote:\n>\n>> After the location of $TRASH_DIRECTORY is adjusted by\n>> $TEST_OUTPUT_DIRECTORY, we go on to use the $test variable to make the\n>> trash directory and cd into it. This means that when\n>> $TEST_OUTPUT_DIRECTORY is not \".\" and an absolute --root has not been\n>> specified, we do not remove the trash directory once the tests are\n>> complete (remove_trash is set to $TRASH_DIRECTORY).\n>> \n>> Fix this by always referring to the trash directory as $TRASH_DIRECTORY.\n>\n> Thanks, this seems to date back all the way to my f423ef5 (tests: allow\n> user to specify trash directory location, 2009-08-09), although I think\n> at that time it was not even possible to run the tests from any other\n> directory. So I am happy to blame Thomas's later patches for violating\n> my assumptions. :)\n>\n> Definitely:\n>\n> Acked-by: Jeff King \n\nIndeed, your blame assignment seems correct :-)\n\nAcked-by: Thomas Rast \n\n> -- >8 --\n> Subject: [PATCH] t/test-lib.sh: drop \"$test\" variable\n>\n> The $test variable is used as an interim buffer for\n> constructing $TRASH_DIRECTORY, and is almost compatible with\n> it (the exception being that $test has not been converted to\n> an absolute path). Let's get rid of it entirely so that\n> later code does not accidentally use it, thinking the two\n> are interchangeable.\n\nAgreed.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"}]}