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

Export multi methods #579

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Gijsreyn
Copy link

PR Summary

Address issue #578. Additional class can be added if needed to have only one export method.

PR Context

@Gijsreyn
Copy link
Author

@SteveL-MSFT Hi Steve, I was actually a bit suprised my change broke the build. The reason is, after debugging and checking out the history on #556, I'm doubting if the change there was fully tested.

If I add the following line in the Invoke-DscOperation:

if ($DesiredState.properties)
{
    # set each property of $dscResourceInstance to the value of the property in the $desiredState INPUT object
    $DesiredState.properties.psobject.properties | ForEach-Object -Process {
        $dscResourceInstance.$($_.Name) = $_.Value
    }
}
($dscResourceInstance | ConvertTo-Json -Depth 10 -Compress) | Write-DscTrace -Operation Trace

I could see this:

image

Clearly the properties are added to the output result set. That's why the tests are failing. Shouldn't these be filtered out?

@anmenaga
Copy link
Collaborator

anmenaga commented Oct 25, 2024

The diagnostic code is checking the wrong thing; 'filtering out' is done by constructing a new hashtable result object.
For 'Get' operation it should be something like:
($Result | ConvertTo-Json -Depth 10 -Compress) | Write-DscTrace -Operation Trace
that is inserted before line 477.

Similarly for 'Export' operation - $Result_obj before line 498.

@@ -484,8 +484,14 @@ function Invoke-DscOperation {
$addToActualState.properties = [psobject]@{'InDesiredState'=$Result}
}
'Export' {
$t = $dscResourceInstance.GetType()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not remove this line;
otherwise $t is $null and you get You cannot call a method on a null-valued expression..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was a bit too aggressive on the removal. The line is added and the test should be working. I shouldn't have made such hasty assumptions. Do you mind running it once again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants