From 67ddc7c55d6b40e3d37b2773e002a50123ae7411 Mon Sep 17 00:00:00 2001 From: Michael Albinus Date: Mon, 14 Sep 2015 22:15:20 +0200 Subject: [PATCH] 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 | 22 ++++++++-------- src/inotify.c | 15 +++++++---- test/automated/file-notify-tests.el | 40 ++++++++++++----------------- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/lisp/filenotify.el b/lisp/filenotify.el index 002c5a5d584..1ec4e13a38b 100644 --- a/lisp/filenotify.el +++ b/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) + (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)))))) + (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) diff --git a/src/inotify.c b/src/inotify.c index b73e8733829..2486563bf9b 100644 --- a/src/inotify.c +++ b/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); diff --git a/test/automated/file-notify-tests.el b/test/automated/file-notify-tests.el index caf9316daa9..3e92925b68a 100644 --- a/test/automated/file-notify-tests.el +++ b/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]."