[WinGet DSC] Add WinGetConfigRoot cmdlet #4990
Conversation
ryfu-msft
changed the title
| } | ||
| else | ||
| { | ||
| [System.Environment]::SetEnvironmentVariable($this.WinGetConfigRoot, "", $this.WinGetScope) |
There was a problem hiding this comment.
Nit: this could set the variable to $null to delete it instead of setting it to the empty string. According to the docs, "" does not delete the variable since .NET 9 (no idea which one this uses...)
| @@ -52,6 +52,12 @@ enum WinGetTrustLevel | |||
| Trusted | |||
| } | |||
|
|
|||
| enum WinGetScope | |||
There was a problem hiding this comment.
Seems like this needs to be a subset of EnvironmentVariableTarget. I didn't know you could use enums like that
Small nit: if it is related to the other enum, I would add a comment for the future
| class WinGetConfigRoot | ||
| { | ||
| [DscProperty(Key)] | ||
| [string]$WinGetConfigRoot = "WinGetConfigRoot" |
There was a problem hiding this comment.
Since this property is settable by a user, there should be a test for what happens if it isn't this specific string.
If it always needs to be that string, then the Path should be the key (since Get() doesn't cause the key to be required, and you would expect Test() and Set() would always have the path, and the string should be an internal string
| $result.Path | Should -Be 'C:\Foo\Bar' | ||
| $result.WinGetScope | Should -Be $_ | ||
|
|
||
| # Remove $WinGetConfigRoot variable. |
There was a problem hiding this comment.
Before you remove it, it may be a good idea to check that if a different Path is specified while the variable exists, then the test returns false.
| $result = InvokeWinGetDSC -Name WinGetConfigRoot -Method Get -Property @{WinGetScope = $_} | ||
| $result.Exist | Should -Be $true | ||
| $result.Path | Should -Be 'C:\Foo\Bar' | ||
| $result.WinGetScope | Should -Be $_ |
There was a problem hiding this comment.
What is the expected result of test if the path is not specified, but the user has requested Exist: true?
| [string]$WinGetConfigRoot = "WinGetConfigRoot" | ||
|
|
||
| [DscProperty()] | ||
| [string]$Path = "" |
There was a problem hiding this comment.
There should be validation that Exist: $true cannot be used with an empty path in Set, otherwise that results in a state conflict where the user has requested a Exist to be true, but the result of the setter will always result in Exist being false
microsoft-github-policy-service
bot
added
the
Needs-Author-Feedback
Added a new dsc resource for ensuring that the $WinGetConfigRoot environment variable exists and is set. Added pester tests to verify that variables are set for both user and machine scope.
Some commands I tried out:
Invoke-DscResource -Name WinGetConfigRoot -Method Get -ModuleName Microsoft.WinGet.DSC -Property @{ Exist=0; Path="C:\Foo\Bar"; WinGetScope='User' }Microsoft Reviewers: Open in CodeFlow