-
Notifications
You must be signed in to change notification settings - Fork 54
Improve speed of Test-IsNanoServer function #154
Conversation
Replace Get-ComputerInfo with Registry Value Test
Codecov Report
@@ Coverage Diff @@
## dev #154 +/- ##
====================================
Coverage ? 83%
====================================
Files ? 19
Lines ? 2760
Branches ? 4
====================================
Hits ? 2303
Misses ? 453
Partials ? 4 |
MSI Integration tests are still failing:
|
Crap... I was worried that would be the case. Sorry for making you wait a month for a fix that didn't actually work (#143). Moving forward though, I think we need to get some better instrumentation into this failing code. Especially given the intermittent nature of this issue, and the fact that it always pops up for certain PR's, even if they don't appear to touch that code, but never pops up for others. I have a couple ideas on how to proceed here, but wanted to get your opinion:
Let me know which route you'd prefer. I'm thinking number 2 would probably be the most efficient, but I'm fine going either way. |
Forgot option 3: YOU add some instrumentation and troubleshoot this yourself. I'm totally fine if you want to volunteer to take that on, but I'm definitely not asking you to. |
Happy for you to take over this PR @mhendric. Good luck! |
Alright, sounds good. I've opened Issue #155 to track this. I'll probably work on the PR offline before attempting to submit, but will submit as soon as tests appear to be passing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
Hey @X-Guardian , I was finally able to get tests passing, and your change looks good. Thanks for your contribution and your patience on this one. |
Pull Request (PR) description
Currently, the Test-IsNanoServer function calls the Get-ComputerInfo cmdlet to check with the computer is running Nano server or not. Get-ComputerInfo can take 40+ seconds to run each time. This function is called for each user and group resource within a Dsc configuration and therefore can make the Dsc application very slow.
This PR replaces Get-ComputerInfo with checking the registry value at HKLM:\Software\Microsoft\Windows NT\CurrentVersion\Server\ServerLevels\NanoServer, as documented here: https://msdn.microsoft.com/en-us/library/windows/desktop/hh846315(v=vs.85).aspx
This Pull Request (PR) fixes the following issues
None
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is