GNU bug report logs - #35418
[PATCH] Don't poll auto-revert files that use notification

Previous Next

Full log

Message #299 received at 35418 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 35418 <at> debbugs.gnu.org
Subject: Re: bug#35418: [PATCH] Don't poll auto-revert files that use
 notification
Date: Sun, 19 May 2019 22:25:57 +0200

[Message part 1 (text/plain, inline)]

19 maj 2019 kl. 11.12 skrev Michael Albinus <michael.albinus <at> gmx.de>:
> 
> Btw, you could always run the four tests on emba.gnu.org. Submit a
> change (also a git branch would do), and the bot on emba.git.org would
> run these test constellations for you. Same for filenotify-tests.

That's good to know -- I'll try that next time.

>>   "Check if auto-revert is active (in current buffer or globally)."
> 
> Remove "or globally".

Done.

>> +(defun auto-revert-test--write-file (string file)
>> +  (write-region string nil file nil 'no-message))
>> +
>> +(defun auto-revert-test--buffer-string (buffer)
>> +  (with-current-buffer buffer
>> +    (buffer-string)))
> 
> Pls add a docstring.

Added.

>> +(ert-deftest auto-revert-test05-global-notify ()
>> +  "Test global-auto-revert-mode without polling."
> 
> Quote `global-auto-revert-mode'. Check also, whether file notification
> is enabled:
> 
> (skip-unless (or file-notify--library
>                 (file-remote-p temporary-file-directory)))

Quoted, and check added.

>> +  (let* ((auto-revert-avoid-polling t)
> 
> Enable file notification explicitly. You don't know, whether the user
> has disabled it.
> 
> (let* ((auto-revert-use-notify t)
>       (auto-revert-avoid-polling t)

Done.

>> +         (auto-revert-interval 2)       ; To speed up the test.
> 
> Do we really want this? I prefer to test the unmodified package. If you
> believe this takes too much time for ordinary "make check" calls, you
> might tag the test case as :expensive-test, like
> auto-revert-test01-auto-revert-several-files and
> auto-revert-test02-auto-revert-deleted-file.

That was probably just me being impatient; I've removed that line and added :expensive-test.

>> +          (global-auto-revert-mode 1)   ; Turn it on.
> 
> Save the value of global-auto-revert-mode, and reset it in Cleanup. You
> don't know the user's settings.

Done.

>> +      (dolist (buf (list buf-1 buf-2 buf-3))
>> +        (when (buffer-live-p buf)
>> +          (kill-buffer buf)))
> 
> Why not (ignore-errors (kill-buffer buf)) ?

Using a condition felt more precise, but I have no strong opinion in this case. Changed to ignore-errors.

> Add the remote test case:
> 
> (auto-revert--deftest-remote auto-revert-test05-global-notify
>  "Test `global-auto-revert-mode' without polling for remote buffers."

Added.

Thanks for the tests and review! Revised patch attached.

[0001-Avoid-polling-in-global-auto-revert-mode-bug-35418.patch (application/octet-stream, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.