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

xSQLServerAlwaysOnAvailabilityGroup: Should use the correct major version SQLPS stubs for unit tests #784

Closed
johlju opened this Issue Aug 25, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@johlju
Contributor

johlju commented Aug 25, 2017

Details of the scenario you tried and the problem that is occurring:
When running unit test for xSQLServerAlwaysOnAvailabilityGroup it should use SQLPS stubs from SQL Server 2014 when major version is 12, and SQLPS stubs from SQL Server 2016 when major version is 13. This would actually test so parameters are not used that are not supported in older versions.

https://github.com/PowerShell/xSQLServer/blob/c290c0074ad34a1a5649b25a475f850008e525f3/Tests/Unit/MSFT_xSQLServerAlwaysOnAvailabilityGroup.Tests.ps1#L86-L88

Currently parameters are added to the existing SQLPS stubs making such test impossible. See for example the stub for New-SqlAvailabilityGroup which contains parameters not in SQL Server 2014.

function New-SqlAvailabilityGroup {
    [CmdletBinding(DefaultParameterSetName='ByPath', ConfirmImpact='Medium')]
    param(
        [Parameter(Mandatory=$true)]
        [ValidateNotNullOrEmpty()]
        [object]
        ${AvailabilityReplica},
 
        [ValidateNotNullOrEmpty()]
        [string[]]
        ${Database},
 
        [object]
        ${AutomatedBackupPreference},
 
        [object]
        ${FailureConditionLevel},
 
        [int]
        ${HealthCheckTimeout},
 
        [Parameter(Mandatory=$true, Position=1)]
        [ValidateNotNullOrEmpty()]
        [string]
        ${Name},
 
        [Parameter(ParameterSetName='ByObject', Mandatory=$true, Position=2, ValueFromPipeline=$true)]
        [ValidateNotNullOrEmpty()]
        [object]
        ${InputObject},
 
        [Parameter(ParameterSetName='ByPath', Position=2)]
        [ValidateNotNullOrEmpty()]
        [string]
        ${Path},
 
        [switch]
        ${Script},
 
        [Parameter()]
        [switch]
        $BasicAvailabilityGroup,
 
        [Parameter()]
        [switch]
        $DatabaseHealthTrigger,
 
        [Parameter()]
        [switch]
        $DtcSupportEnabled
   )
 
    throw '{0}: StubNotImplemented' -f $MyInvocation.MyCommand
}

The DSC configuration that is using the resource (as detailed as possible):
n/a

Version of the Operating System, SQL Server and PowerShell the DSC Target Node is running:
n/a

What module (SqlServer or SQLPS) and which version of the module the DSC Target Node is running:
SQLPS

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

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Aug 25, 2017

Contributor

Maybe add a test helper function that loads the removes SQLPS stubs module and reloads the correct SQLPS stubs module. Making sure the correct one is in the session, so it can be reused in other tests.

Contributor

johlju commented Aug 25, 2017

Maybe add a test helper function that loads the removes SQLPS stubs module and reloads the correct SQLPS stubs module. Making sure the correct one is in the session, so it can be reused in other tests.

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Sep 26, 2017

Contributor

@johlju, do any of the other modules switch the stubs that are used based on the version of SQL being used?

Contributor

randomnote1 commented Sep 26, 2017

@johlju, do any of the other modules switch the stubs that are used based on the version of SQL being used?

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Sep 26, 2017

Contributor

You mean the other resources in this module? No, but I think they should.
Reason for that I added this issue toward this resource is that it will be harder to integration test AG resources, so the unit tests need to handle more. But all resources using SQLPS I think should run unit tests against each version of stub file.

We should also have another stub file for SqlServer module as well.
Then we could run tests for each SQLPS module and also for SQL 2016 and higher we can run unit tests against the SqlServer stubs. But think the SqlServer stub part was filled in another issue.

Contributor

johlju commented Sep 26, 2017

You mean the other resources in this module? No, but I think they should.
Reason for that I added this issue toward this resource is that it will be harder to integration test AG resources, so the unit tests need to handle more. But all resources using SQLPS I think should run unit tests against each version of stub file.

We should also have another stub file for SqlServer module as well.
Then we could run tests for each SQLPS module and also for SQL 2016 and higher we can run unit tests against the SqlServer stubs. But think the SqlServer stub part was filled in another issue.

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Sep 26, 2017

Contributor

Do we need to add a test helper file for this project's tests that will aid with this? If so, where would we store it?

I'm thinking .\Tests\Unit\xSQLServerTestsHelper.psm1 for the file name/location.

Contributor

randomnote1 commented Sep 26, 2017

Do we need to add a test helper file for this project's tests that will aid with this? If so, where would we store it?

I'm thinking .\Tests\Unit\xSQLServerTestsHelper.psm1 for the file name/location.

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Sep 26, 2017

Contributor

Yes. And I suggest you use the same path and filename as here.

https://github.com/PowerShell/xFailOverCluster/blob/dev/Tests/TestHelpers/CommonTestHelper.psm1

The helper functions in this should be moved over here later as well (another issue), but then we have the file already present to add the helper functions to. :)

Contributor

johlju commented Sep 26, 2017

Yes. And I suggest you use the same path and filename as here.

https://github.com/PowerShell/xFailOverCluster/blob/dev/Tests/TestHelpers/CommonTestHelper.psm1

The helper functions in this should be moved over here later as well (another issue), but then we have the file already present to add the helper functions to. :)

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Sep 26, 2017

Contributor

Suggested function name: Import-SQLModuleStub

Contributor

randomnote1 commented Sep 26, 2017

Suggested function name: Import-SQLModuleStub

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Sep 26, 2017

Contributor

Ok, working this into my updates for issue #468.

Contributor

randomnote1 commented Sep 26, 2017

Ok, working this into my updates for issue #468.

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Sep 27, 2017

Contributor

Awesome! The function name works for me.

Contributor

johlju commented Sep 27, 2017

Awesome! The function name works for me.

@johlju johlju added in progress and removed help wanted labels Sep 27, 2017

@johlju johlju closed this in #853 Oct 9, 2017

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

xSQLServerAlwaysOnAvailabilityGroup: Resolved FQDN and Logic Bugs (#853)
- Changes to xSQLServerAlwaysOnAvailabilityGroup
  - Refactored the unit tests to allow them to be more user friendly and to test
    additional SQLServer variations.
    - Each test will utilize the Import-SQLModuleStub to ensure the correct
      module is loaded (issue #784).
  - Fixed an issue when setting the SQLServer parameter to a Fully Qualified
    Domain Name (FQDN) (issue #468).
  - Fixed the logic so that if a parameter is not supplied to the resource, the
    resource will not attempt to apply the defaults on subsequent checks
    (issue #517).
- Added the CommonTestHelper.psm1 to store common testing functions.
  - Added the Import-SQLModuleStub function to ensure the correct version of the
    module stubs are loaded (issue #784).

@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