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

xSQLServerDatabase: Get-TargetResource tests aren't giving accurate results due to scoping. #849

Closed
ChrisLGardner opened this Issue Sep 25, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@ChrisLGardner
Contributor

ChrisLGardner commented Sep 25, 2017

Details of the scenario you tried and the problem that is occurring:

The tests for Get-TargetResource are providing inaccurate results as the second It block in each Context is effectively doing $null | Should Be $null due to the variables being scoped in a different It block.

Version of the DSC module you're using, or 'dev' if you're using current dev branch:

Dev.

I'm working on some changes to this Resource anyway so I'll fix this as part of that (or I can separate it out to different set of changes). I'm not sure if this is done elsewhere so it might effect other resources too.

This is the code, I'll just move the $testParameters assignment and the $result = Get-TargetResource outside the It block so it's part of the Context above it.

It 'Should return the state as present' {
                    $testParameters = $mockDefaultParameters
                    $testParameters += @{
                        Name = 'AdventureWorks'
                    }

                    $result = Get-TargetResource @testParameters
                    $result.Ensure | Should Be 'Present'
                }

                It 'Should return the same values as passed as parameters' {
                    $result.SQLServer | Should Be $testParameters.SQLServer
                    $result.SQLInstanceName | Should Be $testParameters.SQLInstanceName
                    $result.Name | Should Be $testParameters.Name
                }
@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Sep 26, 2017

Contributor

Thanks for finding this.

Instead of moving it outside the It-block, add another $result = Get-TargetResource @testParameters to the second It-block. That is how it is done in other resources, and should be done here as well.

You decide if you can do this change in the same PR as another change, or in a separate PR. Either works for me.

Contributor

johlju commented Sep 26, 2017

Thanks for finding this.

Instead of moving it outside the It-block, add another $result = Get-TargetResource @testParameters to the second It-block. That is how it is done in other resources, and should be done here as well.

You decide if you can do this change in the same PR as another change, or in a separate PR. Either works for me.

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Sep 26, 2017

Contributor

@ChrisLGardner I label this as in progress since you could resolve this.

Contributor

johlju commented Sep 26, 2017

@ChrisLGardner I label this as in progress since you could resolve this.

@ChrisLGardner

This comment has been minimized.

Show comment
Hide comment
@ChrisLGardner

ChrisLGardner Sep 26, 2017

Contributor

That's what I've done.

I'll include it in the PR for adding the collation. Just working out the last few (obvious) bugs now and I'll hopefully have it in for review some time this week.

Contributor

ChrisLGardner commented Sep 26, 2017

That's what I've done.

I'll include it in the PR for adding the collation. Just working out the last few (obvious) bugs now and I'll hopefully have it in for review some time this week.

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Sep 26, 2017

Contributor

Awesome! looking forward to it! 😄

Contributor

johlju commented Sep 26, 2017

Awesome! looking forward to it! 😄

@johlju johlju closed this in #850 Oct 8, 2017

johlju added a commit that referenced this issue Oct 8, 2017

xSQLServerDatabase: Add Collation as a property when defining a Datab…
…ase (#850)

- Changes to xSQLServerDatabase
  - Added parameter to specify collation for a database to be different from server
    collation (issue #767).
  - Fixed unit tests for Get-TargetResource to ensure correctly testing return
    values (issue #849)

@johlju johlju removed the in progress label Oct 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment