PSModule / PATH

A PowerShell module to manage the PATH environment variable on Windows.
http://psmodule.io/PATH/
MIT License
2 stars 0 forks source link

[Bug]: Add-EnvironmentPath is broken #17

Closed koitsu closed 3 months ago

koitsu commented 3 months ago

Description

Was this module ever actually tested before it was uploaded to PSGallery?

Botched use of [System.IO.Path]::PathSeparator is somewhere in this code, resulting in a completely non-functional PATH. My gut feeling is that you are not leaving appending the trailing path separator, so repeated calls results in a PATH that is just one long string.

I would've used https://www.powershellgallery.com/packages/Environment/ except his only offers an add cmdlet, not a removal cmdlet.

Repro:

Get-EnvironmentPath
Add-EnvironmentPath -Path 'D:\bin'
Add-EnvironmentPath -Path 'D:\Util\LAME'
Add-EnvironmentPath -Path 'D:\Util\mkvtoolnix'
Add-EnvironmentPath -Path 'D:\Util\smartmontools\bin'
Add-EnvironmentPath -Path 'D:\Util\Sublime Text'
Add-EnvironmentPath -Path 'D:\Util\Sysinternals'
Add-EnvironmentPath -Path 'D:\Console\cc65\bin'
Get-EnvironmentPath

Result:

PS C:\Users\jdc> Get-EnvironmentPath
C:\Users\jdc\AppData\Local\Microsoft\WindowsApps;
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\bin'
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\Util\LAME'
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\Util\mkvtoolnix'
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\Util\smartmontools\bin'
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\Util\Sublime Text'
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\Util\Sysinternals'
PS C:\Users\jdc> Add-EnvironmentPath -Path 'D:\Console\cc65\bin'
PS C:\Users\jdc> Get-EnvironmentPath
C:\Users\jdc\AppData\Local\Microsoft\WindowsAppsD:\binD:\Util\LAMED:\Util\mkvtoolnixD:\Util\smartmontools\binD:\Util\Sublime TextD:\Util\SysinternalsD:\Console\cc65\bin;
PS C:\Users\jdc>

I've tried other variations too; same result:

@(
'D:\bin',
'D:\Util\LAME',
'D:\Util\mkvtoolnix',
'D:\Util\smartmontools\bin',
'D:\Util\Sublime Text',
'D:\Util\Sysinternals',
'D:\Console\cc65\bin'
) | Add-EnvironmentPath
MariusStorhaug commented 3 months ago

Hi @koitsu, thank you for the bug report!

All modules run tests, just not a good test on this one I think. Having a look at it now to see if I can add the tests that would cover the scenario so I know when its fixed.

What version of the module were you using for this? The latest change (adding [System.IO.Path]::PathSeparator #16 ) was added just 2 days ago, resulting in version 1.0.9.

MariusStorhaug commented 3 months ago

Could you also get the result of [System.IO.Path]::PathSeparator from your environment?

Looking at the tests from the nightly run as well I cannot see this malfunctioning:

koitsu commented 3 months ago

You can test all this yourself. I have moved to another module. My point is that you should really honestly test this stuff yourself. Stop relying on "CI/CD tests" and start doing actual real-world functional tests. You're a programmer, step up to the plate, do not trust automation or unit tests.