-
Notifications
You must be signed in to change notification settings - Fork 7
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
AzureDevOpsDsc: Fixing non-terminating, integration test errors within AzureDevOpsProject
resource
#20
AzureDevOpsDsc: Fixing non-terminating, integration test errors within AzureDevOpsProject
resource
#20
Conversation
…psProject' integration tests.
…sDscResourceBase' class. Also used 'New-InvalidOperationException' when exceptions thrown.
…od when throwing exceptions.
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.
Great work! 🙂 Tiny comments.
Reviewed 5 of 5 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @SphenicPaul)
source/Classes/003.AzDevOpsDscResourceBase.ps1, line 151 at r1 (raw file):
break
We don't need this here since it never gets that far?
source/Classes/003.AzDevOpsDscResourceBase.ps1, line 198 at r1 (raw file):
return [RequiredAction]::Error
We don't need this here since it never gets that far?
… from `AzDevOpsDscResourceBase` class.
…in 'AzDevOpsProject', integration tests.
…s 'False' (and not 'False' or $null)
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.
I've updated a couple of other items I'd noticed - I'd missed specifying the ErrorAction
parameter on one of the test assertions and I've updated a couple of others to ensure 'False' is returned rather than 'False' or $null.
I've rerun these locally against my own, test, AzureDevOps instance and all these pass - Hopefully should be good go go now.
Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @johlju)
source/Classes/003.AzDevOpsDscResourceBase.ps1, line 151 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
break
We don't need this here since it never gets that far?
Done.
source/Classes/003.AzDevOpsDscResourceBase.ps1, line 198 at r1 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
return [RequiredAction]::Error
We don't need this here since it never gets that far?
Done.
…e class to, instead, make use of function to obtain resource name from the inheriting, class/resource.
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 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @SphenicPaul)
tests/Integration/DSCClassResources/AzDevOpsProject.Integration.Tests.ps1, line 205 at r1 (raw file):
Test-DscConfiguration -Verbose -ErrorAction Stop | Should -BeIn @('False',$null)
This should never be able to be anything other than 'True' or 'False', otherwise something else is wrong. Let's add an issue to track this. I can fix that.
Ignore the comment above. I reviewed an older revision. |
@SphenicPaul Awesome work on this. I will push the changes to the temp branch, and see if we can enable the integration tests. |
Pull Request (PR) description
Updated the integration tests to remove non-terminating errors in
AzDevOpsProject
, resource tests whenTest-DscConfiguration
is called:-ErrorAction Stop
to theTest-DscConfiguration
function calls (within the tests) and updated exceptions thrown to make use ofNew-InvalidOperationException
.Test()
function withinAzDevOpsDscResourceBase
class so it always returns a$true
or$false
(and doesn't throw any exception).Additionally, updated 'CONTRIBUTING.md' to add additional environment variable to PowerShell script to enable local execution of integration tests and added link to creating PAT within Azure DevOps.
This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is