DirectoryMock.Move doesn't raise FileSystemWatcher.Renamed if NotifyFilters.DirectoryName is set

[stogle]

Hi. If I have a mock FileSystemWatcherMock with NotifyFilters = NotifyFilters.DirectoryName, and I call DirectoryMock.Move to rename a folder inside the watched location, then the Renamed event isn't raised. It is only raised if I have NotifyFilters = NotifyFilters.FileName set, which is wrong.

I believe this is due to https://github.com/Testably/Testably.Abstractions/blob/main/Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs#L1031:

			ChangeDescription fileSystemChange =
				_fileSystem.ChangeHandler.NotifyPendingChange(WatcherChangeTypes.Renamed,
					container.Type,
					NotifyFilters.FileName,
					destination,
					source);

Here NotifyFilters.FileName is hard-coded, whereas other calls use ToNotifyFilters(container.Type).

---

In a slightly different scenario, if I call DirectoryMock.Move to move a folder out of the watched location, then no event is raised. It should raise the Deleted event in this case. This doesn't seem to be related to NotifyFilters though. It's because FileSystemWatcherMock.MatchesFilter only considers the new path, not the old path:

Testably.Abstractions/Source/Testably.Abstractions.Testing/FileSystem/FileSystemWatcherMock.cs

Lines 359 to 365 in df2e064

	else if (!string.Equals(
	_fileSystem.Execute.Path.GetDirectoryName(changeDescription.Path),
	fullPath,
	_fileSystem.Execute.StringComparisonMode))
	{
	return false;
	}

I don't know if it's just a matter of considering both old and new paths, or it's more complicated than that. Let me know if I should create a separate issue for this.

Thanks again for your efforts.

[vbreuss]

Thanks for reporting.
I will fix both parts of this issue in one PR, so you don't have to split it up, @stogle.

[github-actions]

This is addressed in release v4.3.8.

[stogle]

Hi again @vbreuss. Thank you for the quick response once again. I can confirm that the first scenario is now fixed. The second scenario seems to be fixed when IncludeSubdirectories is true, but for my purposes I don't set that so I still don't see the Deleted event with v4.3.8.

Below I've included two NUnit tests for the scenarios I'm interested in (moving a folder out of a watched folder, and moving a file out of a watched folder). Both fail as written but pass if I add watcher.IncludeSubdirectories = true;. I hope this helps.

[TestFixture]
public class TestablyTests
{
    private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(1);

    private const string Root = @"C:\Temp";
    private const string Path = "Test";
    private const string Folder = "Folder";
    private const string File = "File";

    [Test]
    public async Task DirectoryMockMove_WhenFolderMovedOut_RaisesDeletedEvent()
    {
        // Arrange
        MockFileSystem fileSystem = new();
        fileSystem.Initialize().With(new DirectoryDescription(Root, new DirectoryDescription(Path, new DirectoryDescription(Folder))));
        IFileSystemWatcher watcher = fileSystem.FileSystemWatcher.New(fileSystem.Path.Combine(Root, Path));
        object? source = null;
        FileSystemEventArgs? args = null;
        using SemaphoreSlim handlerInvoked = new(0);
        watcher.Deleted += (s, e) =>
        {
            source = s;
            args = e;
            handlerInvoked.Release();
        };
        watcher.EnableRaisingEvents = true;

        // Act
        fileSystem.Directory.Move(fileSystem.Path.Combine(Root, Path, Folder), fileSystem.Path.Combine(Root, Folder));

        // Assert
        await handlerInvoked.WaitAsync(Timeout, TestContext.CurrentContext.CancellationToken).ConfigureAwait(false);
        Assert.Multiple(() =>
        {
            Assert.That(source, Is.SameAs(watcher));
            Assert.That(args!.ChangeType, Is.EqualTo(WatcherChangeTypes.Deleted));
            Assert.That(args.Name, Is.EqualTo(Folder));
        });
    }

    [Test]
    public async Task FileMockMove_WhenFileMovedOut_RaisesDeletedEvent()
    {
        // Arrange
        MockFileSystem fileSystem = new();
        fileSystem.Initialize().With(new DirectoryDescription(Root, new DirectoryDescription(Path, new FileDescription(File))));
        IFileSystemWatcher watcher = fileSystem.FileSystemWatcher.New(fileSystem.Path.Combine(Root, Path));
        object? source = null;
        FileSystemEventArgs? args = null;
        using SemaphoreSlim handlerInvoked = new(0);
        watcher.Deleted += (s, e) =>
        {
            source = s;
            args = e;
            handlerInvoked.Release();
        };
        watcher.EnableRaisingEvents = true;

        // Act
        fileSystem.File.Move(fileSystem.Path.Combine(Root, Path, File), fileSystem.Path.Combine(Root, File));

        // Assert
        await handlerInvoked.WaitAsync(Timeout, TestContext.CurrentContext.CancellationToken).ConfigureAwait(false);
        Assert.Multiple(() =>
        {
            Assert.That(source, Is.SameAs(watcher));
            Assert.That(args!.ChangeType, Is.EqualTo(WatcherChangeTypes.Deleted));
            Assert.That(args.Name, Is.EqualTo(File));
        });
    }
}

[stogle]

I see you have already reopened this (wow!) so I wanted to mention something I'm noticing, even though I'm not 100% sure what is going on yet.

These cases deal with moving files and directories out of a watched directory - when a Deleted event is expected. I'm also starting to look at moving files in to a watched directory - when a Created event is expected. I'm just getting started, but it looks like I'm getting both a Deleted and a Created event when moving in. I shouldn't get a Deleted event if the location the file was moved from isn't watched.

[stogle]

Looking at the changes to MatchFilters in #866, I think there's two problems.

1. If IncludeSubdirectores is true and changeDescription.ChangeType is Renamed, and changeDescription.OldPath is in the watched path, then the method can return true early, without checking the NotifyFilter or the _filters collection.
2. If IncludeSubdirectories is false and changeDescription.ChangeType is Renamed, we also need to check the OldPath.

I think something like this is needed:

		string fullPath = _fileSystem.Execute.Path.GetFullPath(Path);
		if (IncludeSubdirectories)
		{
			if (!changeDescription.Path.StartsWith(fullPath,
				_fileSystem.Execute.StringComparisonMode))
			{
				if (!(changeDescription.ChangeType == WatcherChangeTypes.Renamed &&
					changeDescription.OldPath?.StartsWith(fullPath,
						_fileSystem.Execute.StringComparisonMode) == true))
				{
					return false;
				}
			}
		}
		else if (!string.Equals(
			_fileSystem.Execute.Path.GetDirectoryName(changeDescription.Path),
			fullPath,
			_fileSystem.Execute.StringComparisonMode))
		{
			if (!(changeDescription.ChangeType == WatcherChangeTypes.Renamed &&
				!string.Equals(
					_fileSystem.Execute.Path.GetDirectoryName(changeDescription.OldPath)
					fullPath,
					_fileSystem.Execute.StringComparisonMode)))
			{
				return false;
			}
		}

[vbreuss]

It seems you already did a thorough analysis. Would you be willing to create a pull request?

I could assign this issue to you...

[stogle]

It seems you already did a thorough analysis. Would you be willing to create a pull request?

I can try.