Re: [PATCH] tests: fix recent regression in tail inotify test

[Pádraig Brady]

On 06/02/15 02:22, Bernhard Voelker wrote:
> On 02/05/2015 05:34 PM, Pádraig Brady wrote:
>> * tests/tail-2/inotify-rotate.sh (cleanup_fail_): Set fail=1
>> so that failures are identified.  Regression in v8.23-63-g111a2b9
>> ---
>>  tests/tail-2/inotify-rotate.sh | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/tail-2/inotify-rotate.sh b/tests/tail-2/inotify-rotate.sh
>> index 452a916..556955f 100755
>> --- a/tests/tail-2/inotify-rotate.sh
>> +++ b/tests/tail-2/inotify-rotate.sh
>> @@ -41,6 +41,7 @@ cleanup_fail()
>>    cat out
>>    warn_ $1
>>    kill $pid
>> +  fail=1
>>  }
>>  
>>  # Perform at least this many iterations, because on multi-core systems
> 
> Good catch! How did you find this one?

While fixing the leak, I tweaked the code
so that it should fail, and was surprised this didn't.
Generally when changing code I try to identify tests in the area,
and tweak the code to verify they fail.

> BTW: the previous implementation used 'fail_ ...' which
> prefixes the error diagnostic with "$ME_: ". Do you think
> it's worth to change it, too??
> 
> -    warn_ $1
> +    warn_ "$ME_: failed test: $@"

Does't really impart any more info. I'll leave that out.

BTW I made another couple of improvements to the test and pushed.

thanks for the review,
Pádraig.

commit 488ee00ca539d2de057534783ec7f7a49cb04c56
Author: Pádraig Brady <address@hidden>
Date:   Thu Feb 5 16:32:49 2015 +0000

    tests: fix recent regression in tail inotify test

    * tests/tail-2/inotify-rotate.sh (cleanup_fail_): Set fail=1
    so that failures are identified.  Regression in v8.23-63-g111a2b9
    Also use print_ver_ rather than open coding --verbose support.
    Also check for more than a single 'b' which seems brittle.

diff --git a/tests/tail-2/inotify-rotate.sh b/tests/tail-2/inotify-rotate.sh
index 452a916..64724f9 100755
--- a/tests/tail-2/inotify-rotate.sh
+++ b/tests/tail-2/inotify-rotate.sh
@@ -16,12 +16,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.

-if test "$VERBOSE" = yes; then
-  set -x
-  tail --version
-fi
-
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ tail

 grep '^#define HAVE_INOTIFY 1' "$CONFIG_HEADER" >/dev/null \
   || expensive_
@@ -41,6 +37,7 @@ cleanup_fail()
   cat out
   warn_ $1
   kill $pid
+  fail=1
 }

 # Perform at least this many iterations, because on multi-core systems
@@ -57,9 +54,9 @@ for i in $(seq 50); do
     timeout 60 tail -s.1 --max-unchanged-stats=1 -F k > out 2>&1 &
     pid=$!

-    echo b > k;
-    # wait for b to appear in out
-    grep_timeout 'b' || { cleanup_fail 'failed to find b in out'; break; }
+    echo 'tailed' > k;
+    # wait for 'tailed' to appear in out
+    grep_timeout 'tailed' || { cleanup_fail 'failed to find "tailed"'; break; }