dsccommunity / xRemoteDesktopSessionHost

This module contains DSC resources for the management and configuration of Microsoft Remote Desktop Session Host (RDSH).
MIT License
35 stars 47 forks source link

MSFT_xRDSessionCollection: Workaround for bug in Windows Server 2019 #92

Closed RobBiddle closed 2 years ago

RobBiddle commented 3 years ago

MSFT_xRDSessionCollection: Workaround for bug in Windows Server 2019

Pull Request (PR) description

Workaround for Get-RDSessionCollection bug in Windows Server 2019.

This Pull Request (PR) fixes the following issues

Fixes #91

Task list


This change is Reviewable

RobBiddle commented 3 years ago

@danielboth Is there anything else I need to do for this PR to be accepted? Any questions about the issue https://github.com/dsccommunity/xRemoteDesktopSessionHost/issues/91 this PR resolves?

danielboth commented 3 years ago

Hey Rob, are you up for writing a test for this PR as well? For example, you could write a test where the call to Get-RDSessionCollection returns 2 results and then ensure that the correct one is picked from that result. Other than that, PR looks good to me.

RobBiddle commented 3 years ago

@danielboth Not sure I follow, that's essentially all I did, selecting the appropriate result if there's more than one Collection. if ($Collection.count -gt 1) { $Collection = $Collection | Where-Object CollectionName -eq $CollectionName }

Are you wanting an additional test case in https://github.com/dsccommunity/xRemoteDesktopSessionHost/blob/95494e5c06dcebf0241244b61403a00cee3b2d2d/tests/Unit/MSFT_xRDSessionCollection.tests.ps1 with mock data containing more than one Collection?

RobBiddle commented 3 years ago

Modified the Mock return to include two Collections. I'm not sure if that's sufficient or if there needs to be another Should statement to check test return value from Get-TargetResource. I'm thinking that might be enough of a test since it was erroring out previously.

nyanhp commented 2 years ago

@danielboth is there anything I can help with to move this PR along? Two of my customers have the same issue, and were able to validate the solution that was proposed here.