Add more functional tests for file change notifications
HaoK opened this issue · 7 comments
We should add some basic end to end coverage of file change notifications on each OS using the actual file watcher. Today most of the tests are using a mock file watcher which doesn't expose any real issues caused by behaviors from the OS.
The configuration reload on change feature has been flaky and although that will have some test coverage of this as well, it would be more appropriate for the basic functionality to be covered in this Repo.
Ideally if there were tests verifying change tokens are fired properly across these scenarios on all supported platforms:
CreateFile
ModifyFile
DeleteFile
DeleteDirectory
DeleteParentDirectory
And a few combinations of the above
Also a rename folder would be interesting.. Especially when you have a pattern watching just the old folder name, just the new file name, or both.
Also if you have a folder like this:
/MyFolder/OtherFolder/Foo.txt
/MyFolder/OtherFolder/Bar.txt
And you have a change token for this: /MyFolder/OtherFolder/Bar.* and another change token for this /Baz/OtherFolder/Bar.*
When you rename MyFolder to Baz - do the appropriate change tokens fire for the files? Technically I am guessing the first token should fire as files have been "deleted" and the second token should fire as files have been "added" - even though it was basically just a folder rename.
The PhysicalFilesWatcher attempts to calculate the old and new paths on rename: https://github.com/aspnet/FileSystem/blob/dev/src/Microsoft.Extensions.FileProviders.Physical/PhysicalFilesWatcher.cs#L186. IMO, the amount of work it does (file enumeration + string replacement) seems a bit daunting especially considering there really isn't a very good way to verify this.
Yeah.. I suppose the worst thing about this current implementation you pointed (if I am reading it correctly) is if the root directory is large and you are only watching a couple of items nested quite deep in the directory, it looks like its going to walk the entire directory doing a little bit of processing to check for a token for each directory item - and after all that work there may only be like 1 token to signal!
I am taking a slightly different approach with change tokens in my InMemory provider. I keep the patterns associated with their change tokens. When a directory item / folder is renamed, I loop through the issued tokens, and if the pattern matches the changed item I signal the token. So basically rather than walking the entire sub directory looking for tokens that "may" exist for each item, I iterate the issued tokens, and evaluate their patterns against the directory item that is new / modified / deleted - if it matches I signal it. This seemed to make more sense to me at the time!
@pranavkm there might be a specific issue with linux file events not firing if a directory and file are created too close together, see aspnet/Configuration#564
Do you know if it's an issue with our wrapper on top of the FSW or the underlying FSW? The latter would warrant starting a thread in https://github.com/dotnet/corefx
This issue was moved to aspnet/Home#2548