Adaot file-notify-tests.el test cases

* lisp/filenotify.el (file-notify-rm-watch): Ignore `file-notify-error'.

* src/inotify.c (Finotify_valid_p): Adapt docstring.

* test/automated/file-notify-tests.el ()
(file-notify-test03-autorevert)
(file-notify-test04-file-validity)
(file-notify-test04-file-validity-remote)
(file-notify-test05-dir-validity)
(file-notify-test05-dir-validity-remote): Adapt docstring.
(file-notify-test04-file-validity): Let events arrive before
calling final `file-notify-valid-p'.  Do not ignore errors.
(file-notify-test05-dir-validity): Do not manipulate
`temporary-file-directory', it isn't necessary.  Let events arrive
before calling final `file-notify-valid-p'.  Do not ignore errors.

lisp/filenotify.el

@@ -345,17 +345,19 @@ DESCRIPTOR should be an object returned by `file-notify-add-watch'."
 
       ;; Call low-level function.
       (when (null (cdr registered))
-	(if handler
-	    ;; A file name handler could exist even if there is no local
-	    ;; file notification support.
-	    (funcall handler 'file-notify-rm-watch desc)
-
-	  (funcall
-	   (cond
-	    ((eq file-notify--library 'gfilenotify) 'gfile-rm-watch)
-	    ((eq file-notify--library 'inotify) 'inotify-rm-watch)
-	    ((eq file-notify--library 'w32notify) 'w32notify-rm-watch))
-	   desc))))))
+        (condition-case nil
+            (if handler
+                ;; A file name handler could exist even if there is no local
+                ;; file notification support.
+                (funcall handler 'file-notify-rm-watch desc)
+
+              (funcall
+               (cond
+                ((eq file-notify--library 'gfilenotify) 'gfile-rm-watch)
+                ((eq file-notify--library 'inotify) 'inotify-rm-watch)
+                ((eq file-notify--library 'w32notify) 'w32notify-rm-watch))
+               desc))
+          (file-notify-error nil))))))
 
 ;; Temporary declarations.
 (defalias 'gfile-valid-p 'identity)

src/inotify.c

@@ -323,12 +323,12 @@ is managed internally and there is no corresponding inotify_init.  Use
 
   watch_descriptor = make_watch_descriptor (watchdesc);
 
-  /* Delete existing watch object. */
+  /* Delete existing watch object.  */
   watch_object = Fassoc (watch_descriptor, watch_list);
   if (!NILP (watch_object))
       watch_list = Fdelete (watch_object, watch_list);
 
-  /* Store watch object in watch list. */
+  /* Store watch object in watch list.  */
   watch_object = Fcons (watch_descriptor, callback);
   watch_list = Fcons (watch_object, watch_list);
 
@@ -351,12 +351,12 @@ See inotify_rm_watch(2) for more information.
     xsignal2 (Qfile_notify_error,
 	      build_string ("Could not rm watch"), watch_descriptor);
 
-  /* Remove watch descriptor from watch list. */
+  /* Remove watch descriptor from watch list.  */
   watch_object = Fassoc (watch_descriptor, watch_list);
   if (!NILP (watch_object))
     watch_list = Fdelete (watch_object, watch_list);
 
-  /* Cleanup if no more files are watched. */
+  /* Cleanup if no more files are watched.  */
   if (NILP (watch_list))
     {
       emacs_close (inotifyfd);
@@ -370,7 +370,12 @@ See inotify_rm_watch(2) for more information.
 DEFUN ("inotify-valid-p", Finotify_valid_p, Sinotify_valid_p, 1, 1, 0,
        doc: /* "Check a watch specified by its WATCH-DESCRIPTOR.
 
-WATCH-DESCRIPTOR should be an object returned by `inotify-add-watch'.  */)
+WATCH-DESCRIPTOR should be an object returned by `inotify-add-watch'.
+
+A watch can become invalid if the file or directory it watches is
+deleted, or if the watcher thread exits abnormally for any other
+reason.  Removing the watch by calling `inotify-rm-watch' also makes
+it invalid.  */)
      (Lisp_Object watch_descriptor)
 {
   Lisp_Object watch_object = Fassoc (watch_descriptor, watch_list);

test/automated/file-notify-tests.el

@@ -321,8 +321,7 @@ Don't wait longer than TIMEOUT seconds for the events to be delivered."
       auto-revert-stop-on-user-input nil)
 
 (ert-deftest file-notify-test03-autorevert ()
-  "Check autorevert via file notification.
-This test is skipped in batch mode."
+  "Check autorevert via file notification."
   (skip-unless (file-notify--test-local-enabled))
   ;; `auto-revert-buffers' runs every 5".  And we must wait, until the
   ;; file has been reverted.
@@ -373,11 +372,10 @@ This test is skipped in batch mode."
       (file-notify--test-cleanup))))
 
 (file-notify--deftest-remote file-notify-test03-autorevert
-  "Check autorevert via file notification for remote files.
-This test is skipped in batch mode.")
+  "Check autorevert via file notification for remote files.")
 
 (ert-deftest file-notify-test04-file-validity ()
-  "Check `file-notify-valid-p'."
+  "Check `file-notify-valid-p' for files."
   (skip-unless (file-notify--test-local-enabled))
   (unwind-protect
       (let ((temporary-file-directory (make-temp-file
@@ -387,7 +385,6 @@ This test is skipped in batch mode.")
                                       file-notify--test-tmpfile
                                       '(change)
                                       #'file-notify--test-event-handler))
-
         (file-notify--test-with-events
             3 3 (lambda (events)
                   (should (equal '(created changed deleted)
@@ -396,25 +393,22 @@ This test is skipped in batch mode.")
           (write-region
            "any text" nil file-notify--test-tmpfile nil 'no-message)
           (should (file-notify-valid-p file-notify--test-desc))
-          (delete-directory temporary-file-directory t)
-          ;; After deleting the parent, the descriptor must not be
-          ;; valid anymore.
-          (should (not (file-notify-valid-p file-notify--test-desc)))))
+          (delete-directory temporary-file-directory t))
+        ;; After deleting the parent, the descriptor must not be valid
+        ;; anymore.
+        (should-not (file-notify-valid-p file-notify--test-desc)))
 
     ;; Exit.
-    (ignore-errors
-      (file-notify--test-cleanup))))
+    (file-notify--test-cleanup)))
 
 (file-notify--deftest-remote file-notify-test04-file-validity
-  "Check `file-notify-valid-p' via file notification for remote
-files.")
+  "Check `file-notify-valid-p' via file notification for remote files.")
 
 (ert-deftest file-notify-test05-dir-validity ()
   "Check `file-notify-valid-p' for directories."
   (skip-unless (file-notify--test-local-enabled))
   (unwind-protect
-      (let ((temporary-file-directory (make-temp-file
-                                       "file-notify-test-parent" t)))
+      (progn
         (setq file-notify--test-tmpfile (file-name-as-directory
                                          (file-notify--test-make-temp-name)))
         (make-directory file-notify--test-tmpfile)
@@ -422,20 +416,18 @@ files.")
                                       file-notify--test-tmpfile
                                       '(change)
                                       #'file-notify--test-event-handler))
-
         (should (file-notify-valid-p file-notify--test-desc))
-        (delete-directory temporary-file-directory t)
-        ;; After deleting the parent, the descriptor must not be
+        (delete-directory file-notify--test-tmpfile t)
+        ;; After deleting the directory, the descriptor must not be
         ;; valid anymore.
-        (should (not (file-notify-valid-p file-notify--test-desc))))
+        (read-event nil nil 0.1)
+        (should-not (file-notify-valid-p file-notify--test-desc)))
 
     ;; Exit.
-    (ignore-errors
-      (file-notify--test-cleanup))))
+    (file-notify--test-cleanup)))
 
 (file-notify--deftest-remote file-notify-test05-dir-validity
-  "Check `file-notify-valid-p' via file notification for remote
-directories.")
+  "Check `file-notify-valid-p' via file notification for remote directories.")
 
 (defun file-notify-test-all (&optional interactive)
   "Run all tests for \\[file-notify]."