[Top][All Lists]

[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: Eli Zaretskii
Subject: 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 <address@hidden>
> Cc: Juanma Barranquero <address@hidden>,  address@hidden
> 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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]