Allow using 'RootModule/NestedModule' as ModuleName #2412
Conversation
fred-gagnon
force-pushed
the
feature/nestedModule
branch
from
8623d0a to
8f3c918
Compare
|
Thanks for the PR. Will review this soon. |
Any idea when it will be reviewed ? |
|
Sorry for the delay. I've been fully occupied at work for a while but will hopefully get some time for OSS this month. I'm also waiting on @nohwnd to return (find time). He'll need to eventually merge and release the pending PRs once approved. |
|
Can you please add a test that uses InModuleScope with this syntax and proves that we ended up in the correct context? E.g. by checking This would also serve as an example usage. |
fred-gagnon
force-pushed
the
feature/nestedModule
branch
from
8f3c918 to
56d25aa
Compare
fred-gagnon
force-pushed
the
feature/nestedModule
branch
from
56d25aa to
509c05b
Compare
|
@nohwnd The tests you requested have been added |
|
@nohwnd Won't this also allow mocking, while cleanup will fail? Haven't had the time to test it yet, but that's a concern I had. |
|
Looks like it would impact Mock, because the function is common. @fred-gagnon can you please allow this only from InModuleScope (e.g. by bool parameter that needs to be provided)? I am not sure what the impact is on mocking, and I won't be able to deeply investigate this any time soon. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -176,7 +176,12 @@ | |||
| if ($PesterPreference.Debug.WriteDebugMessages.Value) { | |||
| Write-PesterDebugMessage -Scope Runtime "Searching for a module $ModuleName." | |||
| } | |||
| $modules = @(& $SafeCommands['Get-Module'] -Name $ModuleName -All -ErrorAction Stop) | |||
| if ($ModuleName -match '(?<RootModule>\w+)[/\\](?<NestedModule>\w+)') { | |||
| $modules = @(& $SafeCommands['Get-Module'] -Name $Matches['RootModule'] -All -ErrorAction Stop | & $SafeCommands['Select-Object'] -ExpandProperty NestedModules | & $SafeCommands['Where-Object'] { $_.Name -eq $Matches['NestedModule'] }) | |||
Check notice
Code scanning / PSScriptAnalyzer
The built-in *-Object-cmdlets are slow compared to alternatives in .NET. To fix a violation of this rule, consider using an alternative like foreach/for-keyword etc.`. Note
| @@ -176,7 +176,12 @@ | |||
| if ($PesterPreference.Debug.WriteDebugMessages.Value) { | |||
| Write-PesterDebugMessage -Scope Runtime "Searching for a module $ModuleName." | |||
| } | |||
| $modules = @(& $SafeCommands['Get-Module'] -Name $ModuleName -All -ErrorAction Stop) | |||
| if ($ModuleName -match '(?<RootModule>\w+)[/\\](?<NestedModule>\w+)') { | |||
| $modules = @(& $SafeCommands['Get-Module'] -Name $Matches['RootModule'] -All -ErrorAction Stop | & $SafeCommands['Select-Object'] -ExpandProperty NestedModules | & $SafeCommands['Where-Object'] { $_.Name -eq $Matches['NestedModule'] }) | |||
Check notice
Code scanning / PSScriptAnalyzer
The built-in *-Object-cmdlets are slow compared to alternatives in .NET. To fix a violation of this rule, consider using an alternative like foreach/for-keyword etc.`. Note
|
@nohwnd The whole point of this PR is for use-cases I have where I need to inject a Mock in a sub-module and there is more than one sub-modules currently loaded which have the same name, for example when 2 REST clients are simultaneously loaded and each one has a Repository sub-module. If I am to limit the use of this change only for InModuleScope, then it is pointless to keep it open. Unless I can use Mock from within InModuleScope? I’ve never tried that and won’t be able to for a few days. |
|
I don't know why I was under the impression that this is for InModuleScope, sorry. To allow this change for Mocking you need to add ton of tests mocks. I am not trying to be difficult, but we rely on simple name to figure out location of mocks. We also don't inspect child modules to clean them etc. So basically no-one knows what the change would cause (see comment: #2235 (comment) ), and your scenario is so rare that it does not seem worth it to break the usage for everyone else. If you want to take it all the way through then I am okay with it, but it won't be easy. 🙂 |
|
Closing due to no activity on this PR, taking it to completion would require the implementation described above, if you want to commit to that please let me know. |
PR Summary
Updated function
Get-CompatibleModuleto allow specifying a module name asRootModule/NestedModule. This change solves an issue where multiple modules with a similarly named nested module cause an exceptionMultiple script or manifest modules named...to be thrown when attempting to create a mock in a nested module.PR Checklist
Create Pull Requestto mark it as a draft. PR can be markedReady for reviewwhen it's ready.