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 <[email protected]> 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; }
