Skip to content
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

Find-DbaInstance - Fix regex for UDP Search #9072

Merged
merged 2 commits into from
Sep 21, 2023
Merged

Find-DbaInstance - Fix regex for UDP Search #9072

merged 2 commits into from
Sep 21, 2023

Conversation

uncletimmy3
Copy link
Contributor

@uncletimmy3 uncletimmy3 commented Sep 4, 2023

fix issues 9071

Please read -- recent changes to our repo

On November 10, 2022, we removed some bloat from our repository (for the second and final time). This change requires that all contributors reclone or refork their repo.

PRs from repos that have not been recently reforked or recloned will be closed and @potatoqualitee will cherry-pick your commits and open a new PR with your changes.

  • Please confirm you have the smaller repo (85MB .git directory vs 275MB or 110MB or 185MB .git directory)

Type of Change

  • Bug fix (non-breaking change, fixes Wrong regex in function Get-SQLInstanceBrowserUDP in Find-DbaInstance command  #9071 )
  • New feature (non-breaking change, adds functionality, fixes # )
  • Breaking change (affects multiple commands or functionality, fixes # )
  • Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
  • Adding code coverage to existing functionality
  • Pester test is included
  • If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
  • Unit test is included
  • Documentation
  • Build system

Purpose

Approach

Commands to test

Screenshots

Learning

@uncletimmy3
Copy link
Contributor Author

# omit some output lines

.\Tests\appveyor.sqlserver.ps1
Creating migration & backup directories
Scenario default
... Running C:\github\dbatools\tests\appveyor.SQL2008R2SP2.ps1
... Setting up AppVeyor Services
... Changing the port on SQL2008R2SP2 to 1433
... Starting SQL2008R2SP2
... Executing startup scripts for SQL Server 2008
Running C:\github\dbatools\tests\appveyor.SQL2016.ps1
... Setting up AppVeyor Services
... Changing the port on SQL2016 to 14333
... Starting SQL2016
... Waiting for SQL Agent to start
... Executing startup scripts for SQL Server 2016

# omit some output lines

Describe : Find-DbaInstance Integration Tests
Context  : Command finds SQL Server instances
Name     : It successfully connects
Result   : Failed
Message  : Expected $true, but got @($true, $true, $true, $true, $true).

Describe : Find-DbaInstance Integration Tests
Context  : Command finds SQL Server instances
Name     : It TcpConnected is true
Result   : Failed
Message  : Expected $true, but got @($true, $true, $true, $false, $true).

Tests

Test checks look strange. It is expected that a boolean variable will be sent into check pipeline, but the test setups two sql instances SQL2008R2SP2 and SQL2016, so array of bools is sent into pipeline.
It is also strange that as a result of the test we received this array @($true, $true, $true, $false, $true), as if there were more instances on the test host.

@uncletimmy3
Copy link
Contributor Author

Perhaps it would be better if we check not the specific value (true or false) of the boolean variables $results.TcpConnected and $results.SqlConnected, but its type, since the main thing for us is that the sql browser response string is parsed correctly. In this case we can check type of $results.TcpConnected and $results.SqlConnected in foreach loop for all sql instances on the test host.

@@ -24,9 +24,9 @@ Describe "$CommandName Integration Tests" -Tag "IntegrationTests" {
Context "Command finds SQL Server instances" {
BeforeAll {
if ($env:APPVEYOR) {
$results = Find-DbaInstance -ComputerName $script:instance3 -ScanType Browser, SqlConnect
$results = Find-DbaInstance -ComputerName $script:instance3 -ScanType Browser, SqlConnect | Select-Object -First 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer this be filtering on the actual $instance3 database service and not randomly picking whatever comes first in the list.

If we do that then the whole if/else block can be removed because it would match no matter the environment.

@potatoqualitee
Copy link
Member

Thanks everyone! Apologies for the delay, that code signing cert debacle took forever but was just resolved moments ago. I'm going to be doing a release today under 2.1.0 and dthis will be included

@potatoqualitee potatoqualitee changed the title Update Find-DbaInstance.ps1 #9071 Find-DbaInstance - Fix regex for UDP Section Sep 21, 2023
@potatoqualitee potatoqualitee changed the title Find-DbaInstance - Fix regex for UDP Section Find-DbaInstance - Fix regex for UDP Search Sep 21, 2023
@potatoqualitee potatoqualitee merged commit c984b2e into dataplat:development Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong regex in function Get-SQLInstanceBrowserUDP in Find-DbaInstance command
3 participants