PowerShellGet: Change AllUsers scope install location to SHARED_MODULES path #5633
Conversation
b8d02ce
to
a01f5e4
Compare
21f0dc4
to
4426696
Compare
| } | ||
| else | ||
| { | ||
| $script:MyDocumentsPSPath = Split-Path -Path ([System.Management.Automation.Platform]::SelectProductNameForDirectory('USER_MODULES')) -Parent |
There was a problem hiding this comment.
we don't really have a way to handle this if you don't have the privilege to write in the directory on non-windows platforms, we can look at extending the test framework for next release.
I recommend a test hook approach, which allows you to provide an alternative location. From a test perspective, the only thing you want to be sure of is when you select a location the files get placed there. It doesn't really matter what that location is, it could be anywhere - A testhook which makes [system.management.automation.platform]::SelectProductNameForDirectory('SHARED_MODULES') point to $TESTDRIVE should be sufficient as long as you have a separate test which returns what the default location of SHARED_MODULES should be.
This provides far more flexibility in the long run
If you can't do that, I would mark this specific test as pending for non-windows
4426696
to
9ea2d3d
Compare
| @@ -154,7 +167,7 @@ Describe "PowerShellGet - Module tests (Admin)" -tags @('Feature', 'RequireAdmin | |||
| $module = Get-Module $ContosoServer -ListAvailable | |||
| $module.Name | Should be $ContosoServer | |||
| $module.ModuleBase | Should Be $installedModuleInfo.InstalledLocation | |||
| } | |||
| } -Skip:$IsLinux | |||
There was a problem hiding this comment.
Please use -Pending instead of -Skip. We track all pending tests because they are supposed to work but now at the time. -Skip means they are not applicable to the platform.
There was a problem hiding this comment.
And also, please add -Pending before the open curly bracket, like
It "Should install a module correctly to the required location with default AllUsers scope" -Pending:$IsLinux {
| @@ -219,7 +232,7 @@ Describe "PowerShellGet - Script tests (Admin)" -tags @('Feature', 'RequireAdmin | |||
| $installedScriptInfo | Should Not Be $null | |||
| $installedScriptInfo.Name | Should Be $FabrikamServerScript | |||
| $installedScriptInfo.InstalledLocation.StartsWith($script:ProgramFilesScriptsPath, [System.StringComparison]::OrdinalIgnoreCase) | Should Be $true | |||
| } | |||
| } -Skip:$IsLinux | |||
9ea2d3d
to
bc24958
Compare
|
In fact, with As for macOS, it turns out the Shared-Module path is not protected, and that's why it passed on macOS. |
|
@adityapatwardhan Can you take a look at the failure in AppVeyor to see what might be wrong? https://ci.appveyor.com/project/PowerShell/powershell/build/v6.1.0-preview.7209 |
|
@bmanikm You didn't address my comment at #5633 (comment) |
7ff19d1
to
6f9e8c9
Compare
…HARED_MODULES location. Refresh PowerShellGet.Tests.ps1 from PowerShell/PowerShellGet repository. PowerShell/PowerShellGet#175
6f9e8c9
to
76fccd5
Compare
…AllUsers scope. (PowerShell#5633) Changed the install location of AllUsers scope on PWSH to SHARED_MODULES location.
…AllUsers scope. (#5633) Changed the install location of AllUsers scope on PWSH to SHARED_MODULES location.
PR Checklist
Note: Please mark anything not applicable to this PR
NA.[feature]if the change is significant or affectes feature testsPR Summary