From 157f817d01133393bf748b38dd84dc5784eee2d8 Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 13:51:53 +0100 Subject: [PATCH 1/7] feat: allow appName override --- src/DotNetPathUtils/PathEnvironmentHelper.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/DotNetPathUtils/PathEnvironmentHelper.cs b/src/DotNetPathUtils/PathEnvironmentHelper.cs index 8d4fe19..7ae5da8 100644 --- a/src/DotNetPathUtils/PathEnvironmentHelper.cs +++ b/src/DotNetPathUtils/PathEnvironmentHelper.cs @@ -18,10 +18,11 @@ internal PathEnvironmentHelper(IEnvironmentService service, string pathVariableN } public PathUpdateResult EnsureApplicationXdgConfigDirectoryIsInPath( - EnvironmentVariableTarget target = EnvironmentVariableTarget.User + EnvironmentVariableTarget target = EnvironmentVariableTarget.User, + string? appName = null ) { - string? appName = _service.GetApplicationName(); + appName ??= _service.GetApplicationName(); if (string.IsNullOrWhiteSpace(appName)) return PathUpdateResult.Error; @@ -42,6 +43,16 @@ public PathUpdateResult EnsureDirectoryIsInPath( { if (string.IsNullOrWhiteSpace(directoryPath)) throw new ArgumentNullException(nameof(directoryPath)); + + try + { + Directory.CreateDirectory(directoryPath); + } + catch (Exception ex) + { + return PathUpdateResult.Error; + } + if ( target == EnvironmentVariableTarget.Process && _pathVariableName.Equals("PATH", StringComparison.OrdinalIgnoreCase) From 31b345dd0e0ac2b69ab60180bfb60eda285a8b64 Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:38:56 +0100 Subject: [PATCH 2/7] test: attempt to fix linux issues --- .../PathEnvironmentHelperTests.cs | 88 ++++++------------- 1 file changed, 28 insertions(+), 60 deletions(-) diff --git a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs index 732cba9..f562539 100644 --- a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs +++ b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs @@ -21,30 +21,21 @@ public PathEnvironmentHelperTests() public async Task EnsureDirectoryIsInPath_When_Path_Does_Not_Exist_Adds_It() { // Arrange - var directoryToAdd = @"C:\MyTool"; - var existingPath = @"C:\ExistingPath"; - var expectedNewPath = $"{existingPath}{Path.PathSeparator}{directoryToAdd}"; + var rootDir = Path.GetPathRoot(Directory.GetCurrentDirectory()) ?? "/"; + var directoryToAdd = Path.Combine(rootDir, "MyTool"); + var existingDir = Path.Combine(rootDir, "ExistingPath"); + var expectedNewPath = $"{existingDir}{Path.PathSeparator}{directoryToAdd}"; - _service.GetFullPath(Arg.Any()).Returns(x => (string)x[0]); // Simple pass-through - _service - .GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User) - .Returns(existingPath); - _service.IsWindows().Returns(true); + _service.GetFullPath(Arg.Any()).Returns(x => (string)x[0]); + _service.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User).Returns(existingDir); + _service.IsWindows().Returns(OperatingSystem.IsWindows()); // Use the real OS for the test // Act - var result = _helper.EnsureDirectoryIsInPath( - directoryToAdd, - EnvironmentVariableTarget.User - ); + var result = _helper.EnsureDirectoryIsInPath(directoryToAdd, EnvironmentVariableTarget.User); // Assert - // await Assert.That(result).IsEqualTo(PathUpdateResult.PathAlreadyExists); await Assert.That(result).IsEqualTo(PathUpdateResult.PathAdded); - - _service - .Received(1) - .SetEnvironmentVariable("PATH", expectedNewPath, EnvironmentVariableTarget.User); - _service.Received(1).BroadcastEnvironmentChange(); + _service.Received(1).SetEnvironmentVariable("PATH", expectedNewPath, EnvironmentVariableTarget.User); } [Test] @@ -173,67 +164,44 @@ public async Task EnsureDirectoryIsInPath_When_Current_Path_Is_Null_Or_Empty_Add ) { // Arrange - var directoryToAdd = @"C:\MyNewTool"; + var rootDir = Path.GetPathRoot(Directory.GetCurrentDirectory()) ?? "/"; + var directoryToAdd = Path.Combine(rootDir, "MyNewTool"); - // Setup the service to return the specified input (null or empty) for the current PATH - _service - .GetEnvironmentVariable("PATH", Arg.Any()) - .Returns(currentPath); - - // Standard setup for mocks + _service.GetEnvironmentVariable("PATH", Arg.Any()).Returns(currentPath); _service.GetFullPath(directoryToAdd).Returns(directoryToAdd); - _service.IsWindows().Returns(true); + _service.IsWindows().Returns(OperatingSystem.IsWindows()); // Use the real OS // Act - var result = _helper.EnsureDirectoryIsInPath( - directoryToAdd, - EnvironmentVariableTarget.User - ); + var result = _helper.EnsureDirectoryIsInPath(directoryToAdd, EnvironmentVariableTarget.User); // Assert - // 1. Verify the operation reported success await Assert.That(result).IsEqualTo(PathUpdateResult.PathAdded); - - // 2. Verify SetEnvironmentVariable was called with *only the new path*, - // since the original was empty. No leading path separator should be present. - _service - .Received(1) - .SetEnvironmentVariable("PATH", directoryToAdd, EnvironmentVariableTarget.User); - - // 3. Verify the environment change was broadcast (since IsWindows is true) - _service.Received(1).BroadcastEnvironmentChange(); + _service.Received(1).SetEnvironmentVariable("PATH", directoryToAdd, EnvironmentVariableTarget.User); } [Test] public async Task EnsureDirectoryIsInPath_When_Existing_Path_Contains_Invalid_Entry_Does_Not_Crash() { // Arrange - var directoryToAdd = @"C:\GoodPath"; - var invalidEntry = @"C:\Bad()) - .Returns(existingPath); - _service.GetFullPath(directoryToAdd).Returns(directoryToAdd); // Normal behavior for the good path - _service.GetFullPath(@"C:\AnotherPath").Returns(@"C:\AnotherPath"); + var rootDir = Path.GetPathRoot(Directory.GetCurrentDirectory()) ?? "/"; + var directoryToAdd = Path.Combine(rootDir, "GoodPath"); + var otherExistingDir = Path.Combine(rootDir, "AnotherPath"); + // We can still use a known invalid character for the test's purpose. + var invalidEntry = otherExistingDir + "<"; + var existingPath = $"{otherExistingDir}{Path.PathSeparator}{invalidEntry}"; + + _service.GetEnvironmentVariable("PATH", Arg.Any()).Returns(existingPath); + _service.GetFullPath(directoryToAdd).Returns(directoryToAdd); + _service.GetFullPath(otherExistingDir).Returns(otherExistingDir); + _service.GetFullPath(invalidEntry).Throws(); // Mock the failure - // Make GetFullPath throw *only* for the invalid entry - _service.GetFullPath(invalidEntry).Throws(); // Act - var result = _helper.EnsureDirectoryIsInPath( - directoryToAdd, - EnvironmentVariableTarget.User - ); + var result = _helper.EnsureDirectoryIsInPath(directoryToAdd, EnvironmentVariableTarget.User); // Assert - // The helper should have gracefully ignored the bad entry and added the new one await Assert.That(result).IsEqualTo(PathUpdateResult.PathAdded); - var expectedNewPath = $"{existingPath}{Path.PathSeparator}{directoryToAdd}"; - _service - .Received(1) - .SetEnvironmentVariable("PATH", expectedNewPath, Arg.Any()); + _service.Received(1).SetEnvironmentVariable("PATH", expectedNewPath, Arg.Any()); } [Test] From bd7b00b79301453cb06fa49f0c7f98a04d2c3f5e Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:39:51 +0100 Subject: [PATCH 3/7] chore: format --- .../PathEnvironmentHelperTests.cs | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs index f562539..c2a5a23 100644 --- a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs +++ b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs @@ -27,15 +27,22 @@ public async Task EnsureDirectoryIsInPath_When_Path_Does_Not_Exist_Adds_It() var expectedNewPath = $"{existingDir}{Path.PathSeparator}{directoryToAdd}"; _service.GetFullPath(Arg.Any()).Returns(x => (string)x[0]); - _service.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User).Returns(existingDir); + _service + .GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User) + .Returns(existingDir); _service.IsWindows().Returns(OperatingSystem.IsWindows()); // Use the real OS for the test // Act - var result = _helper.EnsureDirectoryIsInPath(directoryToAdd, EnvironmentVariableTarget.User); + var result = _helper.EnsureDirectoryIsInPath( + directoryToAdd, + EnvironmentVariableTarget.User + ); // Assert await Assert.That(result).IsEqualTo(PathUpdateResult.PathAdded); - _service.Received(1).SetEnvironmentVariable("PATH", expectedNewPath, EnvironmentVariableTarget.User); + _service + .Received(1) + .SetEnvironmentVariable("PATH", expectedNewPath, EnvironmentVariableTarget.User); } [Test] @@ -167,16 +174,23 @@ public async Task EnsureDirectoryIsInPath_When_Current_Path_Is_Null_Or_Empty_Add var rootDir = Path.GetPathRoot(Directory.GetCurrentDirectory()) ?? "/"; var directoryToAdd = Path.Combine(rootDir, "MyNewTool"); - _service.GetEnvironmentVariable("PATH", Arg.Any()).Returns(currentPath); + _service + .GetEnvironmentVariable("PATH", Arg.Any()) + .Returns(currentPath); _service.GetFullPath(directoryToAdd).Returns(directoryToAdd); _service.IsWindows().Returns(OperatingSystem.IsWindows()); // Use the real OS // Act - var result = _helper.EnsureDirectoryIsInPath(directoryToAdd, EnvironmentVariableTarget.User); + var result = _helper.EnsureDirectoryIsInPath( + directoryToAdd, + EnvironmentVariableTarget.User + ); // Assert await Assert.That(result).IsEqualTo(PathUpdateResult.PathAdded); - _service.Received(1).SetEnvironmentVariable("PATH", directoryToAdd, EnvironmentVariableTarget.User); + _service + .Received(1) + .SetEnvironmentVariable("PATH", directoryToAdd, EnvironmentVariableTarget.User); } [Test] @@ -190,18 +204,25 @@ public async Task EnsureDirectoryIsInPath_When_Existing_Path_Contains_Invalid_En var invalidEntry = otherExistingDir + "<"; var existingPath = $"{otherExistingDir}{Path.PathSeparator}{invalidEntry}"; - _service.GetEnvironmentVariable("PATH", Arg.Any()).Returns(existingPath); + _service + .GetEnvironmentVariable("PATH", Arg.Any()) + .Returns(existingPath); _service.GetFullPath(directoryToAdd).Returns(directoryToAdd); _service.GetFullPath(otherExistingDir).Returns(otherExistingDir); _service.GetFullPath(invalidEntry).Throws(); // Mock the failure // Act - var result = _helper.EnsureDirectoryIsInPath(directoryToAdd, EnvironmentVariableTarget.User); + var result = _helper.EnsureDirectoryIsInPath( + directoryToAdd, + EnvironmentVariableTarget.User + ); // Assert await Assert.That(result).IsEqualTo(PathUpdateResult.PathAdded); var expectedNewPath = $"{existingPath}{Path.PathSeparator}{directoryToAdd}"; - _service.Received(1).SetEnvironmentVariable("PATH", expectedNewPath, Arg.Any()); + _service + .Received(1) + .SetEnvironmentVariable("PATH", expectedNewPath, Arg.Any()); } [Test] From ed3366a0313e90ec143cff53fc7c1699e7adb239 Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:56:38 +0100 Subject: [PATCH 4/7] check env var --- .github/workflows/pr-validation.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index 9ea6656..2eedf6b 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -33,6 +33,9 @@ jobs: with: repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Check Environment Variables + run: 'echo "HOME variable is: [$HOME]" && echo "XDG_CONFIG_HOME is: [$XDG_CONFIG_HOME]"' + - name: Run Linter run: task lint From beef50dcbdee02c1e3c8f1c4ce434764ea3bbdb2 Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 15:05:05 +0100 Subject: [PATCH 5/7] wip --- .../PathEnvironmentHelperTests.cs | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs index c2a5a23..bbe87d4 100644 --- a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs +++ b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs @@ -49,27 +49,18 @@ public async Task EnsureDirectoryIsInPath_When_Path_Does_Not_Exist_Adds_It() public async Task EnsureDirectoryIsInPath_When_Path_Already_Exists_Returns_AlreadyExists() { // Arrange - // 1. Create a root directory that is valid for the current OS. - var rootDir = Path.GetPathRoot(Directory.GetCurrentDirectory()); - if (string.IsNullOrEmpty(rootDir)) - { - // Fallback for non-standard filesystems (e.g. running in certain containers) - rootDir = OperatingSystem.IsWindows() ? @"C:\" : "/"; - } - - // 2. Build platform-agnostic paths using Path.Combine. + // 1. Build platform-agnostic paths + var rootDir = OperatingSystem.IsWindows() ? @"C:\" : "/"; var directoryToAdd = Path.Combine(rootDir, "MyTool"); var otherExistingDir = Path.Combine(rootDir, "ExistingPath"); var existingPath = $"{otherExistingDir}{Path.PathSeparator}{directoryToAdd}"; - // 3. Make the mock behave more realistically for this test. - // It should just return the input, as we are providing "normalized" paths already. + // 2. Setup mocks _service.GetFullPath(Arg.Any()).Returns(x => (string)x[0]); - _service - .GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User) - .Returns(existingPath); + _service.GetEnvironmentVariable("PATH", Arg.Any()).Returns(existingPath); // Act + // 3. THIS IS THE FIX: Ensure we are calling the correct method. var result = _helper.EnsureDirectoryIsInPath( directoryToAdd, EnvironmentVariableTarget.User @@ -77,9 +68,7 @@ public async Task EnsureDirectoryIsInPath_When_Path_Already_Exists_Returns_Alrea // Assert await Assert.That(result).IsEqualTo(PathUpdateResult.PathAlreadyExists); - _service.DidNotReceiveWithAnyArgs().SetEnvironmentVariable(default!, default, default); - _service.DidNotReceive().BroadcastEnvironmentChange(); } [Test] From 5f97adcfed5e7db1bdc7cb4effeb637e59ebec4d Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 15:06:31 +0100 Subject: [PATCH 6/7] chore: format --- src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs index bbe87d4..c455ebf 100644 --- a/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs +++ b/src/DotNetPathUtils.Tests/PathEnvironmentHelperTests.cs @@ -57,7 +57,9 @@ public async Task EnsureDirectoryIsInPath_When_Path_Already_Exists_Returns_Alrea // 2. Setup mocks _service.GetFullPath(Arg.Any()).Returns(x => (string)x[0]); - _service.GetEnvironmentVariable("PATH", Arg.Any()).Returns(existingPath); + _service + .GetEnvironmentVariable("PATH", Arg.Any()) + .Returns(existingPath); // Act // 3. THIS IS THE FIX: Ensure we are calling the correct method. From 65098cf06283349ee0bb8dac6399f366cb531982 Mon Sep 17 00:00:00 2001 From: henry-js <79054685+henry-js@users.noreply.github.com> Date: Wed, 23 Jul 2025 15:43:43 +0100 Subject: [PATCH 7/7] fix: leaky abstraction IO call to CreateDirectory should be mocked --- src/DotNetPathUtils/PathEnvironmentHelper.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/DotNetPathUtils/PathEnvironmentHelper.cs b/src/DotNetPathUtils/PathEnvironmentHelper.cs index 7ae5da8..7f43999 100644 --- a/src/DotNetPathUtils/PathEnvironmentHelper.cs +++ b/src/DotNetPathUtils/PathEnvironmentHelper.cs @@ -31,7 +31,6 @@ public PathUpdateResult EnsureApplicationXdgConfigDirectoryIsInPath( return PathUpdateResult.Error; string appConfigPath = Path.Combine(configHome, appName); - _service.CreateDirectory(appConfigPath); return EnsureDirectoryIsInPath(appConfigPath, target); } @@ -46,7 +45,7 @@ public PathUpdateResult EnsureDirectoryIsInPath( try { - Directory.CreateDirectory(directoryPath); + _service.CreateDirectory(directoryPath); } catch (Exception ex) {