-
-
Notifications
You must be signed in to change notification settings - Fork 803
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
Update tests to be able to run them outside of AppVeyor #9406
Conversation
Context "Gets compatibility for multiple databases" { | ||
$results = Get-DbaDbCompatibility -SqlInstance $script:instance1 | ||
It "Gets results" { | ||
$results | Should Not Be $null | ||
} | ||
Foreach ($row in $results) { | ||
It "Should return Compatiblity level of Version100 for $($row.database)" { | ||
$row.Compatibility | Should Be "Version100" | ||
It "Should return correct compatiblity level for $($row.database)" { |
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.
compatIbility here. may sound nitpicky, but given we're reviewing, we may fix "formalities" as well.
@@ -16,6 +16,7 @@ Describe "$CommandName Unit Tests" -Tag 'UnitTests' { | |||
Describe "$CommandName Integration Tests" -Tags "IntegrationTests" { | |||
BeforeAll { | |||
$server = Connect-DbaInstance -SqlInstance $script:instance2 | |||
$versionName = $server.GetSqlServerVersionName() |
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.
didn't even know this existed, cool.
@@ -15,29 +15,47 @@ Describe "$CommandName Unit Tests" -Tag 'UnitTests' { | |||
|
|||
Describe "$commandname Integration Tests" -Tag "IntegrationTests" { | |||
BeforeAll { | |||
$skip = $false |
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 never really got to use this function and I fear it spun up by potatoqualitee's period on working tirelessly on sharepoint farms. That being said I'm seeing multiple reports about either SQLPS loading DLLs that interfere with us and also DacFx-related issues (many cycling around core-noncore dilemma). That being said I trust you on this change, maybe @potatoqualitee can chime in.
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 dont remember anything and also trust the change 😅
@@ -95,10 +95,14 @@ Describe "$CommandName Integration Tests for Async" -Tags "IntegrationTests" { | |||
} | |||
} | |||
|
|||
# TODO: I think I need some background on this. Was the intention to create the key or not to creeate the key? |
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 concur with the "I don't know" bit
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.
added some comments here and there, in general, LGTM
Thanks for the comments. Will update the PR tomorrow. |
Ok, I will stop here and not add any more commits to get this merged first. But I can roll back individual files if the changes need more discussion. |
beautiful! thank you both 🙇🏼 i will work on that dacfx issue hopefully this week or next. |
I want to run all the tests on a local lab using "real world" sql server instances.
I started with Get-DbaDb*, but try to get all tests working. Current target: getting -DbaDb working.
I learned that some test only work if the instance is on localhost. I think this is not very realistic as we promote to run dbatools on a central admin server, but sometime we need to move files around. Will try to get an overwiew and document this in the tests.
Will update this PR in the next days.