On 11/29/2013 02:43 AM, Pádraig Brady wrote: > It all looks good [...] Thanks for the review(s)!
> [...] apart from the `timeout 2` race. > Unlikely but not something we could release on a bazillion > build hosts doing `make check`. > So as a compromise how about disabling the test by default by adding: > > expensive_ > > I.E. it would only be developers that would be running this, > who might be less likely to hit the issue, or at least > more likely to recognize it and not report false positives. I agree. I found a precedent case in tests/misc/sort-spinlock-abuse.sh. I'll add the following before pushing. Thanks again & have a nice day, Berny diff --git a/tests/rm/r-root.sh b/tests/rm/r-root.sh index aa52bc4..9656274 100755 --- a/tests/rm/r-root.sh +++ b/tests/rm/r-root.sh @@ -32,6 +32,14 @@ skip_if_root_ # LD_PRELOAD environment variable. This requires shared libraries to work. require_gcc_shared_ +# This isn't terribly expensive, but it must not be run under heavy load. +# The reason is the conservative 'timeout' setting below to limit possible +# damage in the worst case which yields a race under heavy load. +# Marking this test as "expensive" therefore is a compromise, i.e., adding +# this test to the list ensures it still gets _some_ (albeit minimal) +# coverage while not causing false-positive failures in day to day runs. +expensive_ + cat > k.c <<'EOF' || framework_failure_ #include <stdio.h> #include <stdlib.h>
