GNU bug report logs - #25539
26.0.50; filenotify-tests.el fails on Windows (32 and 64 bits)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25539 in the body.
You can then email your comments to 25539 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Bug-Gnu-Emacs <bug-gnu-emacs <at> gnu.org>
Subject: 26.0.50; filenotify-tests.el fails on Windows (32 and 64 bits)
Date: Thu, 26 Jan 2017 01:03:09 +0100

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

trunk <at> C:\...\test> mmake -s lisp/filenotify-tests
Testing lisp/filenotify-tests.el
Running 20 tests (2017-01-26 00:36:36+0100)
Library: `w32notify'
   passed   1/20  file-notify-test00-availability
  skipped   2/20  file-notify-test00-availability-remote
   passed   3/20  file-notify-test01-add-watch
  skipped   4/20  file-notify-test01-add-watch-remote
   passed   5/20  file-notify-test02-events
  skipped   6/20  file-notify-test02-events-remote
Reverting buffer `file-notify-test5408q2D'.
Reverting buffer `file-notify-test5408q2D'.
   passed   7/20  file-notify-test03-autorevert
  skipped   8/20  file-notify-test03-autorevert-remote
Test file-notify-test04-file-validity backtrace:
  (if (unwind-protect (setq value-4 (apply fn-2 args-3)) (setq form-de
  (let (form-description-6) (if (unwind-protect (setq value-4 (apply f
  (let ((value-4 (quote ert-form-evaluation-aborted-5))) (let (form-de
  (let ((fn-2 (function file-notify--test-no-descriptors)) (args-3 (li
  file-notify--test-cleanup-p()
  (let ((temporary-file-directory (make-temp-file "file-notify-test-pa
  (unwind-protect (let ((temporary-file-directory (make-temp-file "fil
  (closure (t) nil (let ((fn-194 (function file-notify--test-local-ena
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test file-notify-test04-file-validity "C
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test f
  ert-run-tests(t #[385 "\306 \307\"\203G \211\211G\310U\203 \211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit(nil)
  eval((ert-run-tests-batch-and-exit nil))
  command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/filenotify-tests.el"
  command-line()
  normal-top-level()
Test file-notify-test04-file-validity condition:
    (ert-test-failed
     ((should
       (file-notify--test-no-descriptors))
      :form
      (file-notify--test-no-descriptors)
      :value nil :explanation
      ("Watch descriptor(s) existent:"
       (38542300
"c:/Users/Juanma/AppData/Local/Temp/file-notify-test-parent5408RVW"
                 (nil . file-notify--test-event-handler)))))
   FAILED   9/20  file-notify-test04-file-validity
  skipped  10/20  file-notify-test04-file-validity-remote
Test file-notify-test05-dir-validity backtrace:
  (if (unwind-protect (setq value-4 (apply fn-2 args-3)) (setq form-de
  (let (form-description-6) (if (unwind-protect (setq value-4 (apply f
  (let ((value-4 (quote ert-form-evaluation-aborted-5))) (let (form-de
  (let ((fn-2 (function file-notify--test-no-descriptors)) (args-3 (li
  file-notify--test-cleanup-p()
  (progn (let ((value-269 (cl-gensym "ert-form-evaluation-aborted-")))
  (unwind-protect (progn (let ((value-269 (cl-gensym "ert-form-evaluat
  (closure (t) nil (let ((fn-250 (function file-notify--test-local-ena
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test file-notify-test05-dir-validity "Ch
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test f
  ert-run-tests(t #[385 "\306 \307\"\203G \211\211G\310U\203 \211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit(nil)
  eval((ert-run-tests-batch-and-exit nil))
  command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/filenotify-tests.el"
  command-line()
  normal-top-level()
Test file-notify-test05-dir-validity condition:
    (ert-test-failed
     ((should
       (file-notify--test-no-descriptors))
      :form
      (file-notify--test-no-descriptors)
      :value nil :explanation
      ("Watch descriptor(s) existent:"
       (38542240
"c:/Users/Juanma/AppData/Local/Temp/file-notify-test-parent54084zo"
                 (nil . ignore)))))
   FAILED  11/20  file-notify-test05-dir-validity
  skipped  12/20  file-notify-test05-dir-validity-remote
Test file-notify-test06-many-events backtrace:
  (if (unwind-protect (setq value-4 (apply fn-2 args-3)) (setq form-de
  (let (form-description-6) (if (unwind-protect (setq value-4 (apply f
  (let ((value-4 (quote ert-form-evaluation-aborted-5))) (let (form-de
  (let ((fn-2 (function file-notify--test-no-descriptors)) (args-3 (li
  file-notify--test-cleanup-p()
  (let ((n 1000) source-file-list target-file-list (default-directory
  (unwind-protect (let ((n 1000) source-file-list target-file-list (de
  (closure (t) nil (let ((fn-288 (function file-notify--test-local-ena
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test file-notify-test06-many-events "Che
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test f
  ert-run-tests(t #[385 "\306 \307\"\203G \211\211G\310U\203 \211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit(nil)
  eval((ert-run-tests-batch-and-exit nil))
  command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/filenotify-tests.el"
  command-line()
  normal-top-level()
Test file-notify-test06-many-events condition:
    (ert-test-failed
     ((should
       (file-notify--test-no-descriptors))
      :form
      (file-notify--test-no-descriptors)
      :value nil :explanation
      ("Watch descriptor(s) existent:"
       (38542336
"c:/Users/Juanma/AppData/Local/Temp/file-notify-test-parent5408F-u"
                 (nil . file-notify--test-event-handler)))))
   FAILED  13/20  file-notify-test06-many-events
  skipped  14/20  file-notify-test06-many-events-remote
   passed  15/20  file-notify-test07-backup
  skipped  16/20  file-notify-test07-backup-remote
Test file-notify-test08-watched-file-in-watched-dir backtrace:
  (if (unwind-protect (setq value-4 (apply fn-2 args-3)) (setq form-de
  (let (form-description-6) (if (unwind-protect (setq value-4 (apply f
  (let ((value-4 (quote ert-form-evaluation-aborted-5))) (let (form-de
  (let ((fn-2 (function file-notify--test-no-descriptors)) (args-3 (li
  file-notify--test-cleanup-p()
  (progn (let ((value-370 (cl-gensym "ert-form-evaluation-aborted-")))
  (let* ((--cl-dir-callback-- (function (lambda (event) (let ((file-no
  (unwind-protect (let* ((--cl-dir-callback-- (function (lambda (event
  (closure (t) nil (let ((fn-361 (function file-notify--test-local-ena
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test file-notify-test08-watched-file-in-
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test f
  ert-run-tests(t #[385 "\306 \307\"\203G \211\211G\310U\203 \211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit(nil)
  eval((ert-run-tests-batch-and-exit nil))
  command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/filenotify-tests.el"
  command-line()
  normal-top-level()
Test file-notify-test08-watched-file-in-watched-dir condition:
    (ert-test-failed
     ((should
       (file-notify--test-no-descriptors))
      :form
      (file-notify--test-no-descriptors)
      :value nil :explanation
      ("Watch descriptor(s) existent:"
       (38542360
"c:/Users/Juanma/AppData/Local/Temp/file-notify-test-parent5408RcK"
                 (nil closure ... ... ...)))))
   FAILED  17/20  file-notify-test08-watched-file-in-watched-dir
  skipped  18/20  file-notify-test08-watched-file-in-watched-dir-remote
  skipped  19/20  file-notify-test09-sufficient-resources
  skipped  20/20  file-notify-test09-sufficient-resources-remote

Ran 20 tests, 5 results as expected, 4 unexpected, 11 skipped (2017-01-26
00:38:10+0100)

4 unexpected results:
   FAILED  file-notify-test04-file-validity
   FAILED  file-notify-test05-dir-validity
   FAILED  file-notify-test06-many-events
   FAILED  file-notify-test08-watched-file-in-watched-dir

11 skipped results:
  SKIPPED  file-notify-test00-availability-remote
  SKIPPED  file-notify-test01-add-watch-remote
  SKIPPED  file-notify-test02-events-remote
  SKIPPED  file-notify-test03-autorevert-remote
  SKIPPED  file-notify-test04-file-validity-remote
  SKIPPED  file-notify-test05-dir-validity-remote
  SKIPPED  file-notify-test06-many-events-remote
  SKIPPED  file-notify-test07-backup-remote
  SKIPPED  file-notify-test08-watched-file-in-watched-dir-remote
  SKIPPED  file-notify-test09-sufficient-resources
  SKIPPED  file-notify-test09-sufficient-resources-remote

Makefile:115: recipe for target `lisp/filenotify-tests.log' failed
make[1]: *** [lisp/filenotify-tests.log] Error 1
Makefile:152: recipe for target `lisp/filenotify-tests' failed
make: *** [lisp/filenotify-tests] Error 2

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juanma Barranquero <lekktu <at> gmail.com>,
 Michael Albinus <michael.albinus <at> gmx.de>
Cc: 25539 <at> debbugs.gnu.org
Subject: Re: bug#25539: 26.0.50;
 filenotify-tests.el fails on Windows (32 and 64 bits)
Date: Thu, 26 Jan 2017 18:33:00 +0200

> From: Juanma Barranquero <lekktu <at> gmail.com>
> Date: Thu, 26 Jan 2017 01:03:09 +0100
> 
> 4 unexpected results:
> FAILED file-notify-test04-file-validity
> FAILED file-notify-test05-dir-validity
> FAILED file-notify-test06-many-events
> FAILED file-notify-test08-watched-file-in-watched-dir

The problem is in file-notify--test-cleanup-p, and it happens only
when the parent directory of the file(s) being watched is deleted.

The root cause is that file-notify--test-cleanup-p expects the
notification descriptor(s) to be deleted from the hash table
maintained internally by filenotify.el, when the above happens.  But
that doesn't work on Windows, where deleting the parent directory
simply causes an error whose result is that the thread which watches
the filesystem changes exits abnormally, but the event is not
reported.  So in those cases the descriptor is not removed from the
hash table.

The changes below make the tests succeed, but maybe the above means we
need to augment the w32notify implementation to clean up better in
this case.  Michael?

diff --git a/test/lisp/filenotify-tests.el b/test/lisp/filenotify-tests.el
index d237d0c..5d31251 100644
--- a/test/lisp/filenotify-tests.el
+++ b/test/lisp/filenotify-tests.el
@@ -850,6 +850,8 @@ file-notify--test-with-events
 	;; After deleting the parent directory, the descriptor must
 	;; not be valid anymore.
 	(should-not (file-notify-valid-p file-notify--test-desc))
+        (if (eq system-type 'windows-nt)
+            (file-notify--rm-descriptor file-notify--test-desc))
 
         ;; The environment shall be cleaned up.
         (file-notify--test-cleanup-p))
@@ -906,6 +908,8 @@ file-notify--test-with-events
 	 (file-notify--test-timeout)
 	 (not (file-notify-valid-p file-notify--test-desc)))
         (should-not (file-notify-valid-p file-notify--test-desc))
+        (if (eq system-type 'windows-nt)
+            (file-notify--rm-descriptor file-notify--test-desc))
 
         ;; The environment shall be cleaned up.
         (file-notify--test-cleanup-p))
@@ -975,6 +979,8 @@ file-notify--test-with-events
             (file-notify--test-read-event)
             (delete-file file)))
         (delete-directory file-notify--test-tmpfile)
+        (if (eq system-type 'windows-nt)
+            (file-notify--rm-descriptor file-notify--test-desc))
 
         ;; The environment shall be cleaned up.
         (file-notify--test-cleanup-p))
@@ -1184,6 +1190,9 @@ file-notify--test-with-events
           (delete-directory file-notify--test-tmpfile 'recursive))
         (should-not (file-notify-valid-p file-notify--test-desc1))
         (should-not (file-notify-valid-p file-notify--test-desc2))
+        (when (eq system-type 'windows-nt)
+          (file-notify--rm-descriptor file-notify--test-desc1)
+          (file-notify--rm-descriptor file-notify--test-desc2))
 
         ;; The environment shall be cleaned up.
         (file-notify--test-cleanup-p))

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Juanma Barranquero <lekktu <at> gmail.com>, 25539 <at> debbugs.gnu.org
Subject: Re: bug#25539: 26.0.50;
 filenotify-tests.el fails on Windows (32 and 64 bits)
Date: Fri, 27 Jan 2017 08:54:42 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Eli,

>> 4 unexpected results:
>> FAILED file-notify-test04-file-validity
>> FAILED file-notify-test05-dir-validity
>> FAILED file-notify-test06-many-events
>> FAILED file-notify-test08-watched-file-in-watched-dir
>
> The problem is in file-notify--test-cleanup-p, and it happens only
> when the parent directory of the file(s) being watched is deleted.
>
> The root cause is that file-notify--test-cleanup-p expects the
> notification descriptor(s) to be deleted from the hash table
> maintained internally by filenotify.el, when the above happens.  But
> that doesn't work on Windows, where deleting the parent directory
> simply causes an error whose result is that the thread which watches
> the filesystem changes exits abnormally, but the event is not
> reported.  So in those cases the descriptor is not removed from the
> hash table.
>
> The changes below make the tests succeed, but maybe the above means we
> need to augment the w32notify implementation to clean up better in
> this case.  Michael?

Yes, catching the error somehow in w32notify (don't know how) and fire
up a `stopped' event is preferrable. I let it to you, whether this could
be implemented.

If this is not possible, maybe we shall call
`file-notify--rm-descriptor' in `file-notify-valid-p', when we detect
that the descriptor is not valid anymore, but it still exists.

> @@ -850,6 +850,8 @@ file-notify--test-with-events
>  	;; After deleting the parent directory, the descriptor must
>  	;; not be valid anymore.
>  	(should-not (file-notify-valid-p file-notify--test-desc))
> +        (if (eq system-type 'windows-nt)
> +            (file-notify--rm-descriptor file-notify--test-desc))

It would be OK for me also to apply this patch. The test shall be

(if (string-equal (file-notify--test-library) "w32notify")

Otherwise, we would catch also the remote case when running the test on
MS Windows. And a respective comment might serve why we apply this.

Best regards, Michael.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: lekktu <at> gmail.com, 25539 <at> debbugs.gnu.org
Subject: Re: bug#25539: 26.0.50;
 filenotify-tests.el fails on Windows (32 and 64 bits)
Date: Fri, 27 Jan 2017 10:31:38 +0200

> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Juanma Barranquero <lekktu <at> gmail.com>,  25539 <at> debbugs.gnu.org
> Date: Fri, 27 Jan 2017 08:54:42 +0100
> 
> > The root cause is that file-notify--test-cleanup-p expects the
> > notification descriptor(s) to be deleted from the hash table
> > maintained internally by filenotify.el, when the above happens.  But
> > that doesn't work on Windows, where deleting the parent directory
> > simply causes an error whose result is that the thread which watches
> > the filesystem changes exits abnormally, but the event is not
> > reported.  So in those cases the descriptor is not removed from the
> > hash table.
> >
> > The changes below make the tests succeed, but maybe the above means we
> > need to augment the w32notify implementation to clean up better in
> > this case.  Michael?
> 
> Yes, catching the error somehow in w32notify (don't know how) and fire
> up a `stopped' event is preferrable. I let it to you, whether this could
> be implemented.

I'll leave this bug open, although the test suite now passes.  I think
I see a way of generating 'stopped', but I'll need to test that (and
it will need more changes in filenotify.el and in the tests).

> If this is not possible, maybe we shall call
> `file-notify--rm-descriptor' in `file-notify-valid-p', when we detect
> that the descriptor is not valid anymore, but it still exists.

I don't see how this is possible, as file-notify-valid-p doesn't know
which descriptors are supposed to be removed.  And it will paper over
the real problem, so I'd prefer not to do that.

> > @@ -850,6 +850,8 @@ file-notify--test-with-events
> >  	;; After deleting the parent directory, the descriptor must
> >  	;; not be valid anymore.
> >  	(should-not (file-notify-valid-p file-notify--test-desc))
> > +        (if (eq system-type 'windows-nt)
> > +            (file-notify--rm-descriptor file-notify--test-desc))
> 
> It would be OK for me also to apply this patch. The test shall be
> 
> (if (string-equal (file-notify--test-library) "w32notify")
> 
> Otherwise, we would catch also the remote case when running the test on
> MS Windows. And a respective comment might serve why we apply this.

Done, thanks.

Message #21 received at 25539-done <at> debbugs.gnu.org (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Michael Albinus <michael.albinus <at> gmx.de>, 25539-done <at> debbugs.gnu.org
Subject: Re: bug#25539: 26.0.50; filenotify-tests.el fails on Windows (32 and
 64 bits)
Date: Wed, 2 Oct 2019 00:03:31 +0200

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

On Fri, Jan 27, 2017 at 9:32 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> Done, thanks.

This was marked as Done in 26.1 and it doesn't happen on 27.0.50, so I'm
closing it.

[Message part 2 (text/html, inline)]

Previous Next

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