-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Remove dependency on DNS for Test-Connection
tests on macOS
#12943
Remove dependency on DNS for Test-Connection
tests on macOS
#12943
Conversation
From CI, it seems that solving the setup issue kicks the can down to a test failure instead where |
From the CI, it looks like |
@rjmholt the cmdlet uses the system.net.dns APIs internally quite a bit, so I'd imagine if those APIs themselves are failing it'll most likely fall apart in at least a couple of cases. Definitely seems like something changed that the .NET Core folks will need to take a look at... 😕 |
@@ -234,6 +278,11 @@ Describe "Test-Connection" -tags "CI" { | |||
It "MTUSizeDetect works" -Pending:($env:__INCONTAINER -eq 1) { | |||
$result = Test-Connection $hostName -MtuSize | |||
|
|||
if (-not $?) | |||
{ | |||
Get-Error | Out-String | Write-Host |
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.
Would it be better to use -Because
with the first Should
test to have this in the test log?
Ok the latest change proves that this is a CI configuration issue rather than a .NET issue |
Reopening to run something in CI |
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Management/Test-Connection.Tests.ps1
Outdated
Show resolved
Hide resolved
@SteveL-MSFT Please update your review |
$gatewayAddress = GetGatewayAddress | ||
$publicHostAddress = GetExternalHostAddress -HostName $hostName | ||
|
||
$testAddress = if ($publicHostAddress) { $publicHostAddress } else { $gatewayAddress } |
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.
Not blocking, but could have used the null conditional assignment operator here
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.
This needs to port back to 6.2 -- deliberately removed new operators in most recent commit
Test-Connection
tests on macOS
🎉 Handy links: |
🎉 Handy links: |
PR Summary
Fixes #12935.
macOS has had issues with DNS in CI, so this PR falls back to using the gateway address when DNS doesn't work.
Using the gateway address doesn't seem to work well on other platforms though, so we still try DNS lookup.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.