fix(FileSystem): When creating chained sym links of different types MockFileSystem in windows doesn't throw #833

pw-sgr · 2025-08-11T14:33:48Z

Closes #818

…erent file types on windows

…inks of different types

pw-sgr · 2025-08-11T14:34:47Z

@vbreuss The tests currently fail, because of the Satisfies, I couldn't figure out what I did wrong here, can you fix or tell me what I should do instead?

Edit: To add I currently do not consider FileSystemTypes.DirectoryOrFile, under which condition can a IStorageContainer be of that type?

vbreuss · 2025-08-11T15:33:19Z

@pw-sgr

@vbreuss The tests currently fail, because of the Satisfies, I couldn't figure out what I did wrong here, can you fix or tell me what I should do instead?

You could write it as follows:

await That(() => dirSymLink.ResolveLinkTarget(true))
    .Throws<IOException>()
    .WithMessage($"*{dirSymLink.FullName}*").AsWildcard();

but that would hide the fact, that the actually thrown exception message is
The name of the file cannot be resolved by the system. : 'C:\Windows\Temp\0pfeckbz\d\directoryLinkName5bc87684-062e-4769-a0c3-a2e4bfe1685c' (with a different path). So probably you should rather check for the foll exception message.

Edit: To add I currently do not consider FileSystemTypes.DirectoryOrFile, under which condition can a IStorageContainer be of that type?

I needed the DirectoryOrFile for some internal checks (e.g. for EnumerateFileSystemInfos). An normal container should always be a file or a directory.

vbreuss · 2025-08-11T15:39:55Z

@vbreuss The tests currently fail, because of the Satisfies, I couldn't figure out what I did wrong here, can you fix or tell me what I should do instead?

I created aweXpect/aweXpect#710 for the error with Satisfies.

pw-sgr · 2025-08-11T15:56:28Z

I created aweXpect/aweXpect#710 for the error with Satisfies.

I'll rewrite it to use the working example in the issue and check the type with is then

vbreuss · 2025-08-12T05:24:56Z

I created aweXpect/aweXpect#710 for the error with Satisfies.

I'll rewrite it to use the working example in the issue and check the type with is then

aweXpect v2.21.2 just got released with this fix. So you could update the version in Directory.Packages.props and use the syntax you originally wanted.

pw-sgr · 2025-08-12T05:34:51Z

aweXpect v2.21.2 just got released with this fix. So you could update the version in Directory.Packages.props and use the syntax you originally wanted.

I think WithMessage looks cleaner, will still update the deps, so it could be used in the future

pw-sgr · 2025-08-12T05:39:53Z

One problem I've is that the exception for Directory -> Link is localized on the real file system

pw-sgr · 2025-08-12T07:49:48Z

@vbreuss I've noticed that the actual HResult is -2147024629 instead of the used -2147024773, the problem is that the original IOException is replaced with ExceptionFactory.FileNameCannotBeResolved in FileSystemInfoMock.ResolveLinkTarget. Should I also excempt the HResult -2147024629 there?

Edit: Link to Code

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

Line 355 in 31a3aba

catch (IOException ex) when (ex.HResult != -2147024773)

Edit: Test failure message:

       ↓ (actual)
  "The name of the file cannot be resolved by the system. :…"
  "The directory name is invalid. : 'C:\Windows\Temp\…"
       ↑ (expected)

pw-sgr · 2025-08-12T11:18:47Z

I also noticed that the exception message in Win32Marshal.GetExceptionForWin32Error was changed (EDIT: The message now ends with a dot). Since we don't have a different nuget for .NET 8 and .NET 9 should I use the newer version or the older one?

.NET 8:
https://github.com/dotnet/runtime/blob/a82922ec27a61b1078b1fb41b09076303c7a129d/src/libraries/Common/src/System/IO/Win32Marshal.cs#L63

.NET 9:
https://github.com/dotnet/runtime/blob/4baf26c256b95a9fdc9ba3c25a0a5d0ebc18d96b/src/libraries/Common/src/System/IO/Win32Marshal.cs#L62

vbreuss · 2025-08-12T19:23:03Z

@vbreuss I've noticed that the actual HResult is -2147024629 instead of the used -2147024773, the problem is that the original IOException is replaced with ExceptionFactory.FileNameCannotBeResolved in FileSystemInfoMock.ResolveLinkTarget. Should I also excempt the HResult -2147024629 there?

Edit: Link to Code

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

Line 355 in 31a3aba

catch (IOException ex) when (ex.HResult != -2147024773)

Edit: Test failure message:
       ↓ (actual)
  "The name of the file cannot be resolved by the system. :…"
  "The directory name is invalid. : 'C:\Windows\Temp\…"
       ↑ (expected)

I can't remember why I had to add this, but if you remove the catch-block I am sure there will be a failing test that explains the scenario under which the exception was incorrect. Unfortunately I won't be able to go into more detail before the weekend...

vbreuss · 2025-08-12T19:26:06Z

I also noticed that the exception message in Win32Marshal.GetExceptionForWin32Error was changed (EDIT: The message now ends with a dot). Since we don't have a different nuget for .NET 8 and .NET 9 should I use the newer version or the older one?

.NET 8: https://github.com/dotnet/runtime/blob/a82922ec27a61b1078b1fb41b09076303c7a129d/src/libraries/Common/src/System/IO/Win32Marshal.cs#L63

.NET 9: https://github.com/dotnet/runtime/blob/4baf26c256b95a9fdc9ba3c25a0a5d0ebc18d96b/src/libraries/Common/src/System/IO/Win32Marshal.cs#L62

I expect the behavior of .NET9 to also apply to .NET10, so this will be more relevant in future.

You could

change the behavior depending on #if NET9_0_OR_GREATER directives
if it is only a dot, you could also specify the test without a dot and append .AsPrefix() to the WithMessage() expectation. This way, it checks, that the message starts with the expected string, but can have an additional dot.

… points to file

…low an optional dot in the exception message

…leSystemInfoMock

pw-sgr · 2025-08-13T06:26:40Z

I can't remember why I had to add this, but if you remove the catch-block I am sure there will be a failing test that explains the scenario under which the exception was incorrect. Unfortunately I won't be able to go into more detail before the weekend...

I've removed it and executed the tests that reference it locally, pushed it to also verify it works here. Can revert it later on if that works for you

pw-sgr · 2025-08-13T07:34:12Z

I think I found the missing piece. In InMemoryStorage.ResolveFinalLinkTarget it throws ExceptionFactory.FileNameCannotBeResolved but doesn't set the appropriate HResult. This exception is catched in FileSystemInfoMock.ResolveLinkTarget and rethrown with the correct HResult. I've moved the HResult set to InMemoryStorage.ResolveFinalLinkTarget

The failing tests now succeed on my machine (they also failed by me, I haven't executed the tests for FileSystemInfo as I overlooked it)

@vbreuss is my assumption correct or was the catch needed for something not tested?

pw-sgr · 2025-08-13T07:39:52Z

I also noticed that the exception message in Win32Marshal.GetExceptionForWin32Error was changed (EDIT: The message now ends with a dot). Since we don't have a different nuget for .NET 8 and .NET 9 should I use the newer version or the older one?
.NET 8: https://github.com/dotnet/runtime/blob/a82922ec27a61b1078b1fb41b09076303c7a129d/src/libraries/Common/src/System/IO/Win32Marshal.cs#L63
.NET 9: https://github.com/dotnet/runtime/blob/4baf26c256b95a9fdc9ba3c25a0a5d0ebc18d96b/src/libraries/Common/src/System/IO/Win32Marshal.cs#L62

I expect the behavior of .NET9 to also apply to .NET10, so this will be more relevant in future.

You could
* change the behavior depending on `#if NET9_0_OR_GREATER` directives

* if it is only a dot, you could also specify the test without a dot and append `.AsPrefix()` to the `WithMessage()` expectation. This way, it checks, that the message starts with the expected string, but can have an additional dot.

Used the pre-processor to concate the dot and used Regex for the message, making the dot optional, so allows specifically the optional dot and nothing more after

vbreuss · 2025-08-13T10:10:32Z

@vbreuss is my assumption correct or was the catch needed for something not tested?

I remember adding this catch block due to a failing test. It is an ugly solution, so if you found a better way and the tests succeed I am more than happy with it 😀

Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs

vbreuss · 2025-08-13T13:30:37Z

One problem I've is that the exception for Directory -> Link is localized on the real file system

How did you solve this issue? Is there something we can do in the test suite?

pw-sgr · 2025-08-13T18:39:34Z

One problem I've is that the exception for Directory -> Link is localized on the real file system

How did you solve this issue? Is there something we can do in the test suite?

I haven't so far. I see it as a fine detail and I wanted everything to work before I get to it. Mostly those translated messages come from somewhere internal of private lib, runtime or interop. So it's rather tricky to get them.

…annotBeResolved with a wrong HResult

…ctory name invalid

pw-sgr · 2025-08-14T06:41:33Z

So I looked some more into the translating message 'The directory name was invalid.' and it came from the PInvoke error 267. I used Marshal.GetPInvokeErrorMessage to "translate" the message. The marshal method is only available for .NET 7 and above, so for .NET Framework 4.8 I used the untranslated message. The if for the symbolic link seems to already not be set if .NET Framework 4.8 but I just wanted to be sure

pw-sgr · 2025-08-14T07:12:24Z

Ready to merge if no comments

vbreuss

I really like that you got rid of the try/catch block in FileSystemInfoMock and the overall direction of the change looks great.

Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs

Tests/Testably.Abstractions.Tests/FileSystem/DirectoryInfo/ResolveLinkTargetTests.cs

Tests/Testably.Abstractions.Tests/FileSystem/FileInfo/ResolveLinkTargetTests.cs

Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs

…tern

…ssDenied and removed pre-processor statement

pw-sgr · 2025-08-15T06:57:19Z

Your request should now all be applied, please check that I've haven't forgotten some (did some weird git locally)

vbreuss

The change looks really good. I have one small issue with the scope of an IDisposable. Could you have a quick look and then we can finish the PR...

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

Copilot

Pull Request Overview

This PR fixes a bug where MockFileSystem on Windows doesn't properly throw exceptions when creating chained symbolic links of different types (file vs directory), making the behavior consistent with the real Windows file system.

Adds proper type checking for symbolic link chains to throw appropriate exceptions on Windows
Updates test cases to verify the new behavior with proper platform-specific exception handling
Simplifies exception handling by consolidating access denied logic

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`Tests/Testably.Abstractions.Tests/FileSystem/FileInfo/ResolveLinkTargetTests.cs`	Added new test for cross-type symbolic links and improved existing tests with proper error validation
`Tests/Testably.Abstractions.Tests/FileSystem/DirectoryInfo/ResolveLinkTargetTests.cs`	Added new test for cross-type symbolic links and improved existing tests with proper error validation
`Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs`	Implemented type checking logic to throw proper exceptions when resolving mixed-type symbolic link chains
`Source/Testably.Abstractions.Testing/Storage/InMemoryContainer.cs`	Updated to use consolidated access denied exception method
`Source/Testably.Abstractions.Testing/Helpers/ExceptionFactory.cs`	Consolidated access denied exception creation and removed platform-specific restriction
`Source/Testably.Abstractions.Testing/FileSystem/FileSystemInfoMock.cs`	Simplified exception handling by removing redundant try-catch block
`Directory.Packages.props`	Updated aweXpect package version from 2.21.0 to 2.21.1

Copilot · 2025-08-15T11:02:26Z

Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs

 		{
 			throw ExceptionFactory.FileNameCannotBeResolved(
-				originalLocation.FullPath);
+				originalLocation.FullPath, _fileSystem.Execute.IsWindows ? -2147022975 : -2146232800


The hardcoded HResult values (-2147022975 and -2146232800) should be defined as named constants to improve readability and maintainability. Consider creating constants like WINDOWS_TOO_MANY_LINKS_HRESULT and UNIX_TOO_MANY_LINKS_HRESULT.

Copilot · 2025-08-15T11:02:26Z

Tests/Testably.Abstractions.Tests/FileSystem/FileInfo/ResolveLinkTargetTests.cs

-		await That(() => link.ResolveLinkTarget(true)).Throws<IOException>();
+		await That(Act).Throws<IOException>()
+			.WithHResult(Test.RunsOnWindows ? -2147022975 : -2146232800).And
+			.WithMessageContaining($"'{fileInfo.FullName}'");


The hardcoded HResult values (-2147022975 and -2146232800) are duplicated across test files. Consider defining these as shared constants in a test utilities class to ensure consistency and easier maintenance.

Copilot · 2025-08-15T11:02:26Z

Tests/Testably.Abstractions.Tests/FileSystem/DirectoryInfo/ResolveLinkTargetTests.cs


-		await That(() => link.ResolveLinkTarget(true)).Throws<IOException>();
+		await That(Act).Throws<IOException>()
+			.WithHResult(Test.RunsOnWindows ? -2147022975 : -2146232800).And


The hardcoded HResult values (-2147022975 and -2146232800) are duplicated across test files. Consider defining these as shared constants in a test utilities class to ensure consistency and easier maintenance.

Suggested change

.WithHResult(Test.RunsOnWindows ? -2147022975 : -2146232800).And

.WithHResult(Test.RunsOnWindows ? TestConstants.HResultTooManySymbolicLinksWindows : TestConstants.HResultTooManySymbolicLinksNonWindows).And

github-actions · 2025-08-15T11:48:51Z

This is addressed in release v4.3.2.

pw-sgr added 2 commits August 11, 2025 16:31

fix(FileSystem): fixed ResolveLinkTarget not throwing when using diff…

3c172d9

…erent file types on windows

test(FileSystem): added tests in ResolveLinkTargetTests for chained l…

f7673ff

…inks of different types

pw-sgr changed the title ~~[WIP] When creating chained sym links of different types targeting the other type MockFileSystem in windows doesn't throw~~ [WIP] fix(FileSystem): When creating chained sym links of different types MockFileSystem in windows doesn't throw Aug 11, 2025

pw-sgr added 3 commits August 12, 2025 07:36

chore: update aweXpect to 2.21.1

4650540

fix: fixed IOException when resolving link target not having hresult

ce6c8db

test: fixed and expanded tests in ResolveLinkTargetTests

03eb7f3

pw-sgr changed the title ~~[WIP] fix(FileSystem): When creating chained sym links of different types MockFileSystem in windows doesn't throw~~ fix(FileSystem): When creating chained sym links of different types MockFileSystem in windows doesn't throw Aug 12, 2025

pw-sgr added 3 commits August 13, 2025 08:21

fix: fixed the IOException information thrown when directory sym link…

8f1dd5c

… points to file

test: fixed test ResolveLinkTarget_OfDifferentTypes_ShouldThrow to al…

02c9828

…low an optional dot in the exception message

refactor: removed FileNameCannotBeResolved exception wrapping from Fi…

5ac6bab

…leSystemInfoMock

vbreuss reviewed Aug 13, 2025

View reviewed changes

Source/Testably.Abstractions.Testing/Storage/InMemoryStorage.cs Outdated Show resolved Hide resolved

pw-sgr added 2 commits August 14, 2025 07:35

fix: fixed InMemoryStorage.ResolveFinalLink target throwing FileNameC…

c9742ec

…annotBeResolved with a wrong HResult

test: improved tests for IFileInfo and IDirectoryInfo ResolveLinkTarget

3f4956b

pw-sgr added 2 commits August 14, 2025 08:37

fix: fixed InMemoryStorage.ThrowOnLinkTypeChange not translating dire…

2c47dba

…ctory name invalid

test: fixed test for IDirectoryInfo.ResolveLinkTarget

2525706

vbreuss requested changes Aug 14, 2025

View reviewed changes

pw-sgr added 3 commits August 15, 2025 08:09

refactor: replaced Marshal error message InMemoryStorage

56ed6f9

test: refactored ResolveLinkTargetTests to use Arrange-Act-Assert pat…

b3cff08

…tern

refactor: renamed ExceptionFactory.UnixFileModeAccessDenied into Acce…

445e41a

…ssDenied and removed pre-processor statement

pw-sgr requested a review from vbreuss August 15, 2025 06:56

vbreuss reviewed Aug 15, 2025

View reviewed changes

Source/Testably.Abstractions.Testing/FileSystem/FileSystemInfoMock.cs Show resolved Hide resolved

vbreuss approved these changes Aug 15, 2025

View reviewed changes

Source/Testably.Abstractions.Testing/FileSystem/FileSystemInfoMock.cs Show resolved Hide resolved

mergify bot merged commit e7f364e into Testably:main Aug 15, 2025
12 checks passed

vbreuss requested a review from Copilot August 15, 2025 11:01

Copilot AI reviewed Aug 15, 2025

View reviewed changes

github-actions bot added the state: released label Aug 15, 2025

dependabot bot mentioned this pull request Aug 15, 2025

Bump the all-deps group with 4 updates AlamoEngine-Tools/EaW-Modinfo-Implementation#259

Closed

	.WithHResult(Test.RunsOnWindows ? -2147022975 : -2146232800).And
	.WithHResult(Test.RunsOnWindows ? TestConstants.HResultTooManySymbolicLinksWindows : TestConstants.HResultTooManySymbolicLinksNonWindows).And

Uh oh!

fix(FileSystem): When creating chained sym links of different types MockFileSystem in windows doesn't throw #833

fix(FileSystem): When creating chained sym links of different types MockFileSystem in windows doesn't throw #833

Uh oh!

Conversation

pw-sgr commented Aug 11, 2025

Uh oh!

pw-sgr commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbreuss commented Aug 11, 2025

Uh oh!

vbreuss commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pw-sgr commented Aug 11, 2025

Uh oh!

vbreuss commented Aug 12, 2025

Uh oh!

pw-sgr commented Aug 12, 2025

Uh oh!

pw-sgr commented Aug 12, 2025

Uh oh!

pw-sgr commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pw-sgr commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbreuss commented Aug 12, 2025

Uh oh!

vbreuss commented Aug 12, 2025

Uh oh!

pw-sgr commented Aug 13, 2025

Uh oh!

pw-sgr commented Aug 13, 2025

Uh oh!

pw-sgr commented Aug 13, 2025

Uh oh!

vbreuss commented Aug 13, 2025

Uh oh!

Uh oh!

vbreuss commented Aug 13, 2025

Uh oh!

pw-sgr commented Aug 13, 2025

Uh oh!

pw-sgr commented Aug 14, 2025

Uh oh!

pw-sgr commented Aug 14, 2025

Uh oh!

vbreuss left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pw-sgr commented Aug 15, 2025

Uh oh!

vbreuss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 15, 2025

pw-sgr commented Aug 11, 2025 •

edited

Loading

vbreuss commented Aug 11, 2025 •

edited

Loading

pw-sgr commented Aug 12, 2025 •

edited

Loading

pw-sgr commented Aug 12, 2025 •

edited

Loading

vbreuss left a comment •

edited

Loading