Test the checks

[michalporeba]

While the issue #316 is still being discussed I have started writing integration tests for the database checks.

Please review the current approach.

[wsmelton]

I'll leave it to @potatoqualitee and @SQLDBAWithABeard but I don't believe the test themselves should include calls to dbatools. If dbatools module breaks the test I think it should cause the test to fail, and not the test itself...if you know my meaning.

As well, use of the module itself will cause the test on Appveyor to run longer than really needed compared to just using T-SQL.

In that regard I don't believe $psitem will be of use in testing against the checks. So the Set-DatabaseForIntegrationTesting.ps1 could be moved into this file with a few lines of code instead of trying to use the database command.

If you want to check the test we have in dbatools that will be the general pattern we use for testing here as well I believe. I see no reason to not keep the same naming convention for objects also.

[potatoqualitee]

I think we should use dbatools commands - the reason we don't within dbatools is because it is dbatools. But since it'll all be tested, we as a dependent module can use it.

[michalporeba]

I was the one who wanted to stop using dbatools for the testing, but only in the checks, so I can run it against my instances with hundreds of databases. The tests of checks I imagine will be run against one or two specific databases and in that case the performance is good enough. Sticking to dbatools also means we are using tested methods to modify configuration and hopefully that means the likelyhood of false results due to problems with the test setup will be limited. I think we should stick with dbatools unless we will see there are problems with Appveyor run times.

As for the Set-DatabaseForIntegrationTesting.ps1 I see it more clearly as an internal function that can be shared between scripts. We may need to create test databases when testing other check files, while the one I had in the Database.Tests.ps1 is clearly useful only in that single file. I think the $psitem is useful for testing against multiple instances. I know we cannot test different versions at the appveyor, but I will be testing it locally too.

I'll check the dbatools tests.

Thanks for the review.

[michalporeba]

As for the test convetions I had a look at the dba tools and here is what I think

1. The file naming convention will cause a bit awkward name like Database.Tests.Tests.ps1. That's why I tried Test-Database.Tests.ps1 but then it looks too much like a test for a command we don't have (yet) so I suggest to stick with my Test.Database.Tests.ps1 or with the awkward Database.Tests.Tests.ps1
2. Looping through a list of instances and using $psitem or hardoding localhost for testing. As much as I can see how it is sufficient when testing in dedicated appveyor agents, it doesn't help in a 'home' setup where I will have to use named instances to validate against different versions. It is especially important for the T-SQL approach to getting the management information from the instance. The new DMVs have been added in every version and it might be that for some checks we will have to revert to a SMO approach to support 2008.
3. I see in some of the tests you use UnitTests and IntegrationTests tags. That's really a good idea. I'll follow suit.
4. Another idea I like is limiting the scope of variables used to $script: to make sure they don't interfere with things being tested. I like it.

Other than those two, I don't think I'm far off from what I see in dbatools.

[SQLDBAWithABeard]

Not altering anything that will affect current tests, adding functionality for the future. Happy to move this forward