Make the '-SslProtocol' tests pending #5605
Conversation
markekraus
left a comment
markekraus
left a comment
There was a problem hiding this comment.
This covered IWR, but what about the IRM failing tests?
There was a problem hiding this comment.
This is all very hard to read and follow what is doing what. If we are going to use a foreach anyway, we might as well have them structured like this:
$testCases1 = @(
@{Test = @{SslProtocol = 'Default'; ActualProtocol = 'Default'}; Pending = $false }
@{Test = @{SslProtocol = 'Tls'; ActualProtocol = 'Tls'}; Pending = $false }
@{Test = @{SslProtocol = 'Tls11'; ActualProtocol = 'Tls11'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls12'; ActualProtocol = 'Tls12'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls12'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls12'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls11'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls11'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls11'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls'}; Pending = $IsMacOS }
@{Test = @{SslProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls12'}; Pending = -not $IsWindows }
)
foreach ($entry in $testCases1) {
It "Verifies Invoke-WebRequest -SslProtocol <SslProtocol> works on <ActualProtocol>" -TestCases ($entry.test) -Pending:($entry.Pending) {
param($SslProtocol, $ActualProtocol)
$params = @{
Uri = Get-WebListenerUrl -Test 'Get' -Https -SslProtocol $ActualProtocol
SslProtocol = $SslProtocol
SkipCertificateCheck = $true
}
$response = Invoke-WebRequest @params
$result = $Response.Content | ConvertFrom-Json
$result.headers.Host | Should Be $params.Uri.Authority
}
}There was a problem hiding this comment.
Ah, forgot the Invoke-RestMethod tests 😨 Will add soon.
There was a problem hiding this comment.
Some of the tests are currently not run in macOS. I thought they didn't apply to macOS.
Mark a test pending means the test should pass but it doesn't, and thus need to be fixed.
Mark a test Skip means it doesn't apply to a platform, and it's expected that the test doesn't run on that platform.
Do you think all test cases should work on all platforms? Or some tests should be marked as -Skip on some platforms?
There was a problem hiding this comment.
I originally did the test this way because I was trying to avoid a forereach loop. at the time the logic was simpler.. but.. it continues to get more complex 😦 .
In a perfect world, these should all work on all platforms. CoreFX does not have a any of these issues we've run into here documented. by all Accounts macOS should support multiple protocols at once (either they need to document that it's not supported or this is an issue) , and that one that fails on Linux should work. I need to open issues with CoreFX, but... I have make simple C# repros and I haven't had the time.
There was a problem hiding this comment.
OK, I will update the change to mark the tests -Pending on the platforms they currently don't run.
Thanks @markekraus !
markekraus
left a comment
markekraus
left a comment
There was a problem hiding this comment.
LGTM. My requested changes are not blocking.
There was a problem hiding this comment.
Can you change this to possible CoreFX issue, please? My own missing word it bugging me 😆
There was a problem hiding this comment.
Fixed. Thanks for spotting it!
There was a problem hiding this comment.
Same as above, if you would be so kind.
|
@daxian-dbw please fix, merge conflict. |
daxian-dbw
force-pushed
the
disablessl
branch
from
59a800b to
51380b8
Compare
daxian-dbw
force-pushed
the
disablessl
branch
from
51380b8 to
02ffdc9
Compare
4 participants
Related to #5590
Make the
-SslProtocoltests pending for now.