[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#25539: 26.0.50; filenotify-tests.el fails on Windows (32 and 64 bits
From: |
Michael Albinus |
Subject: |
bug#25539: 26.0.50; filenotify-tests.el fails on Windows (32 and 64 bits) |
Date: |
Fri, 27 Jan 2017 08:54:42 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) |
Eli Zaretskii <eliz@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.