xRDSessionCollection: refactoring and adding features

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

xRDSessionCollection: refactoring and adding features #80

Closed svenboll closed 3 years ago

svenboll commented 4 years ago

Pull Request (PR) description

complete refactor of resource xRDSessionCollection by adding feature "add or remove a sessionhost to/from a collection"
add feature "set parameter RDSessionHost->NewConnectionAllowed"

Task list

[x] Added an entry under the Unreleased section of file CHANGELOG.md. Entry should say what was changed, and how that affects users (if applicable).
[x] Resource documentation added/updated in README.md.
[x] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
[x] Comment-based help added/updated.
[x] Localization strings added/updated in all localization files as appropriate.
[ ] Examples appropriately added/updated.
[ ] Unit tests added/updated. See DSC Community Testing Guidelines.
[ ] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
[ ] New/changed code adheres to DSC Community Style Guidelines.

This change is

danielboth commented 4 years ago

Hi @svenboll, thank you for this PR!

I noticed there are a few issues in the build pipeline for this DSC resource at this moment which I need to resolve, as currently, the tests are failing to run properly. I hope to fix that this week, after which I will get back to you on this PR.

danielboth commented 4 years ago

Hi @svenboll, the tests are now running properly again and showing some issues on the code in your PR, this mainly relates to the DSC style guidelines. While these might not be perfect, it at least makes sure all code for DSC resources is formatted in the same way. Please check here for an overview (including line numbers): https://dev.azure.com/dsccommunity/xRemoteDesktopSessionHost/_build/results?buildId=2437&view=logs&j=47266b63-9d03-5281-d38b-75133a4264f6&t=4d8b314c-6b76-5ae4-aed5-2779b7a3afdb&l=273.

Next to that, are you up to writing unit tests for your changes as well?

svenboll commented 4 years ago

Hi @danielboth , thanks for your comments. I will check the changes and look at the style guidelines. After that i try to write some new unit tests, i never done it before, but let me have a look at it.

danielboth commented 4 years ago

Ok, let me know if you need any help on writing the tests!

One question from me on this "NewConnectionAllowed" parameter that we are able to set to "NotUntilReboot". What happens when, after applying a DSC configuration, we reboot the system? I would assume that Get-RDSessionHost then returns "Yes" for NewConnectionAllowed, instead of "NotUntilReboot", so my question is, to what level is "NotUntilReboot" a desired state?

When set to NotUntilReboot, after a reboot, DSC would (depending on the settings of the LCM) just try to convert it back to NotUntilReboot. Which is not the state you would like to end up in I assume.

svenboll commented 4 years ago

@danielboth That's a good objection, a desired state with a value "notuntilreboot" will always be restored after the computer is rebooted. I just checked the values that are valuable for that entry and added them to be configured. But you are right, it doesn't makes sense for a desired state configuration. I think i will remove it.

For testing my pester tests, is there a quick-and-easy-to-use variant of testing them locally?

danielboth commented 4 years ago

Hi @svenboll, yes there is a quick-and-easy-to-use variant of running your tests locally.

From the root of the repository just run:

.\build.ps1 -Tasks test

svenboll commented 3 years ago

no time to update this PR, i will close it and open a new one in the future