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

Fixes issues discussed in #2786 #2794

Merged
merged 65 commits into from
Mar 3, 2023
Merged

Fixes issues discussed in #2786 #2794

merged 65 commits into from
Mar 3, 2023

Conversation

salbeck-sit
Copy link
Contributor

Pull Request (PR) description

This PR implements several fixes to the DSC-resource AADAdministrativeUnit
The resource has been updated to only use the v1.0 Graph-profile
Note that the schema and parameters are slightly changed with respect to parameter ScopedRoleMembers in order to avoid nested CimInstances. However, the current resource is broken and has never worked so this isn't really a BR. Please see #2786

This Pull Request (PR) fixes the following issues

@salbeck-sit
Copy link
Contributor Author

An additional commit is required, Export-TargetResource doesn't work. Please wait.

@salbeck-sit
Copy link
Contributor Author

Export-TargetResource still not fully working

@salbeck-sit
Copy link
Contributor Author

I've submitted a PR for ReverseDSC as I had issues getting rid of commas between CimInstances in arrays when exporting resources.. That PR should be approved before this one

@salbeck-sit salbeck-sit requested a review from desmay as a code owner March 1, 2023 12:20
@salbeck-sit
Copy link
Contributor Author

rebased, update should be OK now

Copy link
Member

@ykuijs ykuijs left a comment

Choose a reason for hiding this comment

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

LGTM

@ykuijs ykuijs closed this Mar 1, 2023
@ykuijs ykuijs reopened this Mar 1, 2023
@salbeck-sit
Copy link
Contributor Author

Export breaks now - not yet sure why

@ykuijs
Copy link
Member

ykuijs commented Mar 2, 2023

I recently updated the unit tests to specifically check if output is returned. So the code should always return a value. If the code is hitting an exception and therefore jumping into the catch block, it returns an empty string. This will trigger the test to fail.

To troubleshoot: Open the test file and place a breakpoint on the line where the export is ran. Than execute the test and step through the code. You can see where the code fails and jumps into the catch block. When you then run $_, you can see what the error is that is being generated.

@salbeck-sit
Copy link
Contributor Author

@ykuijs, thanks a lot ! I've never thought to run tests like that but it was invaluable in this case.
It turns out I'd made an error in the mock of New-DSCConnection so it returned 'Credential' instead of 'Credentials'. Small 's', big difference...

@ykuijs
Copy link
Member

ykuijs commented Mar 3, 2023

Ahhhh, that small 's' has been a pain in the b*tt on my end as well while updating all unit tests. That mistake was made on multiple locations.

I just started the unit tests and will merge it once they complete. Thanks a lot for all your help!

Copy link
Member

@ykuijs ykuijs left a comment

Choose a reason for hiding this comment

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

LGTM

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