STATUS:OK is reported before internal delay is finished

[individual-it]

When the client touches a file, the change notifications are blocked for some time but the status of the file is already reported as STATUS:OK. This happens e.g. when a new account is connected and all files are synced down.

The responsible code is

client/src/libsync/syncengine.cpp

Lines 61 to 74 in 9117275

	/** When the client touches a file, block change notifications for this duration (ms)
	*
	* On Linux and Windows the file watcher can't distinguish a change that originates
	* from the client (like a download during a sync operation) and an external change.
	* To work around that, all files the client touches are recorded and file change
	* notifications for these are blocked for some time. This value controls for how
	* long.
	*
	* Reasons this delay can't be very small:
	* - it takes time for the change notification to arrive and to be processed by the client
	* - some time could pass between the client recording that a file will be touched
	* and its filesystem operation finishing, triggering the notification
	*/
	static const std::chrono::milliseconds s_touchedFilesMaxAgeMs(3 * 1000);

For the GUI tests this is an issue, because as soon as STATUS:OK is reported back by the socket API the tests assume that the next step can be carried out. If the tests change the file during this delay, the client ignores the change and does not upload the changed file.

In #8814 a static wait had to be implemented to make sure that the file is ready to be changed.

Ideally STATUS:OK would only be reported AFTER the delay is over, both for the folder as well as the files. But it also would be sufficient if the status of the root folder would be something different than OK during the delay.

This would help the tests to act correctly and not to waste time even if the delay would change in the future.

[TheOneRing]

STATUS:OK Is meant for applications waiting for the availability of the file and for the status icon of the file and not designed for testing.

If a sync is enforced we would still detect the change to the file.

[individual-it]

With the end-to-end tests we trying to rely only on things that a user could do. A user would wait till the status icon is indicating that the file is synced and then change the content of it. The only thing the tests do different is checking the status through the socket API and not watching the visual icon. And the tests are obviously faster than a user. It's unlikely (but possible) that a user would overwrite a file within the first 3sec.

STATUS:OK Is meant for applications waiting for the availability of the file

That is exactly what we are trying to do. The End-to-End test-suite is the application waiting for the availability of the file. As soon as the socket API says the file is available, it get changed, but then the sync does not work. So strictly speaking STATUS:OK indicates that the file was available for read but not for write. If that is what it is supposed to indicate, then the feature works as intended, but the tests would need an other solution to find out if a file is ready to be written.

We could keep the static waits, but that would lead to the issues I described above.

Or we do a force-sync. That would cost a lot of time (because AFAIK it cannot be done by soket-API, but need to be done through UI clicks) and even more important it would not test what we are intending to test. We want to test if the client picks up and uploads a file change correctly. Using the force-sync would not test that behavior.

[TheOneRing]

Again it is ok to directly write to that file, it will just not be directly synced.

I agree that the timeout behaviour is not optimal but at least for now there is nothing we can change easily.
For testing we could emit a signal once the file is watched again but 🤷‍♀️

[TheOneRing]

@dragotin I think we should clear the touched list once we finished the propagation, what do you think?

[TheOneRing]

@dragotin we actually clear the list of touched files https://github.com/owncloud/client/blob/master/src/libsync/syncengine.cpp#L109 but only 30s after the sync... any idea why?

[TheOneRing]

Hmm 28c12a3

Could the file watcher be so slow that it takes 30s to detect all changes we induced?

[dragotin]

The reasoning for the whole touchFilesList are summarized here: https://github.com/owncloud/client/blob/master/src/libsync/syncengine.cpp#L61

Actually I think the 30 seconds after the propagation is finished is really long...

OTOH, I don't think it really play a role because in slotAddTouchedFile(const QString &fn) there is a check that removes entries that are older than the 3 seconds from the list. So the 30 seconds thing is really a general cleanup, when nothing else happens.

[github-actions]

This issue was marked stale because it has been open for 30 days with no activity. Remove the stale label or comment or this will be closed in 7 days.

[github-actions]

The issue was marked as stale for 7 days and closed automatically.