On Thu, Apr 13, 2017 at 7:57 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Apr 13, 2017 at 10:55:08AM -0700, Stefan Beller wrote:
>
>> On Thu, Apr 13, 2017 at 9:37 AM, Jeff King <peff@xxxxxxxx> wrote:
>> > Ah, OK, that makes more sense. I can detect it reliably by just checking
>> >
>> > ! test -d $root/trash*
>> >
>> > in my stress-test after the test successfully completes.
>>
>> Would we want to report such an error from the test suite itself?
>> (My line of thinking is that this is a similar issue to broken && chains,
>> which we do report).
A broken &&-chain can harm the test's correctness by hiding errors.
The failing 'rm -rf $trash' during housekeeping, after all the tests
in the test script has been run and their results reported... not
really, I would think, though arguably it's a sign that something is
fishy.
> Yeah, I had a similar thought. I can't think of any reason why it would
> trigger a false positive, as long as we account for test-failure and the
> --debug flag properly. I don't think we can just drop the "-f" from the
> final "rm", because it may be overriding funny permissions left by
> tests.
FWIW, I used the following rather simple change during stress-testing
these patches (barring white-space damage):
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 13b569682..d7fa15a69 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -763,7 +763,7 @@ test_done () {
test -d "$remove_trash" &&
cd "$(dirname "$remove_trash")" &&
- rm -rf "$(basename "$remove_trash")"
+ rm -rf "$(basename "$remove_trash")" || exit 1
test_at_end_hook_