Fix PSVersion in PSSessionConfiguration tests#5554
Fix PSVersion in PSSessionConfiguration tests#5554adityapatwardhan merged 3 commits intoPowerShell:masterfrom
Conversation
89e8a2d to
594e17a
Compare
| $Result.Session.Name | Should be $TestSessionConfigName | ||
| $Result.Session.SessionType | Should be "Default" | ||
| $Result.Session.PSVersion | Should be 6.0 | ||
| $Result.Session.PSVersion | Should BeExactly $expectedPSVersion |
There was a problem hiding this comment.
I am still not convinced changing this from a string literal to the variable is the right move. Please provide answers to the question in the other PR.
There was a problem hiding this comment.
The tests should work on all PowerShell Core versions without changes.
We don't pass-through the version constant, we evaluate it. So we should have a contract for this in the tests.
|
|
||
| $Result.Session.Name | Should be $TestSessionConfigName | ||
| $Result.Session.PSVersion | Should be 6.0 | ||
| $Result.Session.PSVersion | Should BeExactly $expectedPSVersion |
| $Result.Session.UseSharedProcess | Should be $UseSharedProcess | ||
| } | ||
|
|
||
| It "Validate Set-PSSessionConfiguration -PSVersion" { |
There was a problem hiding this comment.
This test is very similar to the test at L504. Can you use -testcases?
There was a problem hiding this comment.
I think it's fine to keep it as is. This Context is for Set-PSSessionConfiguration and the one above is in the Context of Register-PSSessionConfiguration.
|
|
||
| $LocalConfigFilePath = CreateTestConfigFile | ||
|
|
||
| $expectedPSVersion = "$($PSVersionTable.PSVersion.Major)`.$($PSVersionTable.PSVersion.Minor)" |
There was a problem hiding this comment.
`.
The backtick is not necessary.
| Unregister-PSSessionConfiguration -Name $TestSessionConfigName -Force -NoServiceRestart -ErrorAction SilentlyContinue | ||
| } | ||
|
|
||
| $expectedPSVersion = "$($PSVersionTable.PSVersion.Major)`.$($PSVersionTable.PSVersion.Minor)" |
There was a problem hiding this comment.
I think this should be moved to the BeforeAll block of the Describe, otherwise the use of $expectedPSVersion below from a different Context block won't see the variable.
And the backtick is not necessary.
|
|
||
| $Session.Name | Should be $TestSessionConfigName | ||
| $Session.PSVersion | Should Be 5.1 | ||
| } |
There was a problem hiding this comment.
Please fix the indentation of this it block.
| $Result.Session.UseSharedProcess | Should be $UseSharedProcess | ||
| } | ||
|
|
||
| It "Validate Set-PSSessionConfiguration -PSVersion" { |
There was a problem hiding this comment.
I think it's fine to keep it as is. This Context is for Set-PSSessionConfiguration and the one above is in the Context of Register-PSSessionConfiguration.
|
@iSazonov Please add the |
|
I will add the |
2bab0cd to
1ef532a
Compare
|
|
||
| It "Validate Set-PSSessionConfiguration -PSVersion" { | ||
|
|
||
| Set-PSSessionConfiguration -Name $TestSessionConfigName -PSVerion 5.1 |
There was a problem hiding this comment.
should be -PSVersion (missing an s)
|
Restarted job on Travis-CI as one test failed due to intermittent failure. |
|
Filed #5567 for travis-ci test failure |
|
Not taking for GA because this was caused by a change that is only in |
Check that we can register and change (Set-) remote endpoint
configurations with given PSVersion.