Fixing bug #2607 related to ModuleSpec syntax in RequiredModules - #3594
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
does requiredModuleSpecification only needs to be added when currentModule is not null?
this is confusing here. if so, please add comment explains why
Sorry, something went wrong.
There was a problem hiding this comment.
Originally I was thinking that both addition should be bundled together; but thinking more about it - it is better to move the new addition to its own 'if' check. Updated.
Sorry, something went wrong.
There was a problem hiding this comment.
please also check that the module is actually loaded.
Sorry, something went wrong.
There was a problem hiding this comment.
Good point; updated.
Sorry, something went wrong.
|
good to sign off |
Sorry, something went wrong.
…ell#3594) This fixes issue PowerShell#2607. 'RequiredModules' is a field in module manifest that can reference other modules using ModuleSpecification format. The basic version of this format (just module name) was working fine, however, there was a problem when a more detailed version of the format was used (the one that uses module versions or/and GUIDs). During module import, there is a check for cyclic references through 'RequiredModules' field. The bug was in this check for cyclic references, related to comparison rules for ModuleSpecification objects - as a result, the code was incorrectly reporting 'cyclic reference' error in cases when there was none. Also, added tests for different ModuleSpecification formats and a test for error when there is actually a cyclic reference.
This fixes issue #2607.
'RequiredModules' is a field in module manifest that can reference other modules using ModuleSpecification format.
The basic version of this format (just module name) was working fine, however there was a problem when more detailed version of the format was used (the one that uses module versions or/and GUIDs).
During module import, there is a check for cyclic references through 'RequiredModules' field. The bug was in this check for cyclic references , related to comparison rules for ModuleSpecification objects - as a result code was incorrectly reporting 'cyclic reference' error in cases when there was none.
Added tests for different ModuleSpecification formats and a test for error when there is actually a cyclic reference.
Test results before the fix:
Test results after the fix:
