Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore-TestEnvironment: New PSModulePath in target Machine is overwritten #127

Closed
johlju opened this issue May 17, 2023 · 3 comments · Fixed by #128
Closed

Restore-TestEnvironment: New PSModulePath in target Machine is overwritten #127

johlju opened this issue May 17, 2023 · 3 comments · Fixed by #128
Labels
enhancement The issue is an enhancement request.

Comments

@johlju
Copy link
Member

johlju commented May 17, 2023

The root cause is that

  1. Initialize-TestEnvironment saves the current machine environment variable PSModulePath and then updates it with the path to the built module.

$oldPSModulePath = $env:PSModulePath
if ($null -ne $oldPSModulePath)
{
$oldPSModulePathSplit = $oldPSModulePath.Split([io.path]::PathSeparator)
}
else
{
$oldPSModulePathSplit = $null
}
if ($oldPSModulePathSplit -ccontains $moduleParentFilePath)
{
# Remove the existing module path from the new PSModulePath
$newPSModulePathSplit = $oldPSModulePathSplit | Where-Object { $_ -ne $moduleParentFilePath }
}
else
{
$newPSModulePath = $oldPSModulePath
}
$RequiredModulesPath = Join-Path -Path $moduleParentFilePath 'RequiredModules'
if ($newPSModulePathSplit -cnotcontains $RequiredModulesPath)
{
$newPSModulePathSplit = @($RequiredModulesPath) + $newPSModulePathSplit
}
$newPSModulePathSplit = @($moduleParentFilePath) + $newPSModulePathSplit
$newPSModulePath = $newPSModulePathSplit -join [io.Path]::PathSeparator
Set-PSModulePath -Path $newPSModulePath

  1. The tests run for SqlSetup and the installation sets a new path to SQLPS in machine environment variable PSModulePath.

https://github.com/dsccommunity/SqlServerDsc/blob/f9bde8a61230754c078257d115103726f749570a/tests/Integration/DSC_SqlSetup.Integration.Tests.ps1#L171-L175

if ($TestEnvironment.OldPSModulePath -ne $env:PSModulePath)
{
Set-PSModulePath -Path $TestEnvironment.OldPSModulePath
if ($TestEnvironment.TestType -in ('Integration','All'))
{
# Restore the machine PSModulePath for integration tests.
Set-PSModulePath -Path $TestEnvironment.OldPSModulePath -Machine
}
}

  1. After the tests finishes the command Restore-TestEnvironment reverts the machine environment variable PSModulePath to the one saved in step 1 overwriting the new path that was set in step 2.
  2. All other tests no longer finds the module SQLPS and fails.

Originally posted by @johlju in dsccommunity/SqlServerDsc#1932 (comment)

@johlju johlju transferred this issue from dsccommunity/SqlServerDsc May 17, 2023
@johlju
Copy link
Member Author

johlju commented May 17, 2023

There is also a bug that is essentially not reverting the correct state. The variable$script:MachineOldPSModulePath is set here:

if (!$script:MachineOldPSModulePath)
{
Write-Warning "This will change your Machine Environment Variable"
$script:MachineOldPSModulePath = [System.Environment]::GetEnvironmentVariable('PSModulePath', 'Machine')
}

Is not passed here:

# Return the test environment
return @{
DSCModuleName = $Module
Module = $ModuleUnderTest
DSCResourceName = $DscResourceName
TestType = $TestType
ImportedModulePath = $moduleToImportFilePath
OldPSModulePath = $oldPSModulePath
OldExecutionPolicy = $oldExecutionPolicy
}

So it is never used here:

if ($TestEnvironment.TestType -in ('Integration','All'))
{
# Restore the machine PSModulePath for integration tests.
Set-PSModulePath -Path $TestEnvironment.OldPSModulePath -Machine
}

@johlju
Copy link
Member Author

johlju commented May 17, 2023

Before reverting the PSModulePath in the machine state it should first look if there have been new paths added to the machine state (other than the built module path) and make sure to keep them after reverting to previous state.

@johlju
Copy link
Member Author

johlju commented May 17, 2023

We could add this functionality behind an optional parameter KeepNewMachinePSModulePaths to avoid a breaking change. It could be integration tests that actually sets paths that must be reverted.

@johlju johlju added enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community. in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels May 17, 2023
@johlju johlju removed the in progress The issue is being actively worked on by someone. label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is an enhancement request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant