bug#21435: 25.0.50; file-notify has problems after renames

[Tassilo Horn]

Michael Albinus <michael.albinus@gmx.de> writes:

>> That already revealed two problems:
>>
>>   1. Now `file-notify-test02-events-remote' fails because after every
>>      expected `changed' event an additional `attribute-changed' event is
>>      received.  This is wrong because when adding the watch, only
>>      '(change) is given as FLAGS argument, not '(change
>>      attribute-change).
>
> I'll contact the Tramp maintainer about :-)

Deliver him my best wishes. :-)

>>   2. When I change the watch FLAGS to '(change attribute-change), there
>>      are still no attribute-changed events received in the local case.
>
> attribute-change happens when you change file permissions, or
> modification time w/o changing the file contents. Something like

Ah, ok, so when you write to a file you'll only get a `changed' event,
and not an additional `attribute-changed' event for the changed
modification time.  So basically, attribute changes are subsumed by
`changed' and `created' events.

By the way, I think it could be hard to test `attribute-changed' events
because those probably depend on the filesystem and mount options on the
machine where the tests are run, e.g., if access time recording is
enabled or not.

>> And a question: Will the events read by
>> `file-notify--wait-for-events' still be processed by the handler
>> function?
>
> Yes, they should.

Ok, good.

>> And what's the intention of (file-notify--wait-for-events 5
>> file-notify--test-results)?  The timeout of 5 is reasonable, but the
>> UNTIL argument here just defines that it waits until the very first
>> of possibly up to nine yet missing events is awaited here, or do I
>> get something wrong?
>
> Well, this is an open point here.  When waiting for events, you don't
> know how many events will arrive for a given file operation.  See the
> first command above, "echo", it is good for 2 events.

In the tests we know that our test files do not exist initially, so I
expect the first write to a file to result in a `created' followed by an
`changed' event.  And for other operations I have similar expectations,
so in the test02 I know that exactly nine events should be received
(unless my expectations are wrong).

> If you know of a better check for "alle events we wait for have
> arrived" - go on.

In the concrete case where I know it should be nine events, I could use
(= 9 (length file-notify--test-events)) as the UNTIL argument.

I've added a new macro to the tests now which lets you do things this
way:

--8<---------------cut here---------------start------------->8---
        ;; Check creation, change, and deletion.
        (file-notify--test-with-events
            3 3 (lambda (events)
                  (should (equal '(created changed deleted)
                                 (mapcar #'cadr events))))
          (write-region
           "any text" nil file-notify--test-tmpfile nil 'no-message)
          (delete-file file-notify--test-tmpfile))
--8<---------------cut here---------------end--------------->8---

This means we're waiting for 3 events for at most 3 seconds, and then
apply the lambda to the received events.  The rest is the code which
causes the events to be emitted.

Another thing: the remote tests, especially the test03-autorevert one,
take really, really long (maybe 30 seconds).  I saw that this uses some
mock TRAMP method which suggests it is a mockup connection which can
probably simulate a fast or a slow connection.  If so, I'd prefer to
have a reasonably fast one so that I don't try to avoid running all
tests frequently.

Bye,
Tassilo