Fix PSVersion in PSSessionConfiguration tests #5554

iSazonov · 2017-11-27T06:26:16Z

Check that we can register and change (Set-) remote endpoint
configurations with given PSVersion.

markekraus · 2017-11-27T12:06:37Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -488,7 +492,7 @@ namespace PowershellTestConfigNamespace

                    $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


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.

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.

markekraus · 2017-11-27T12:06:37Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -557,7 +570,7 @@ namespace PowershellTestConfigNamespace
                    $Result = [PSObject]@{Session = (Get-PSSessionConfiguration -Name $TestSessionConfigName) ; Culture = (Get-Item WSMan:\localhost\Plugin\microsoft.powershell\lang -ea SilentlyContinue).value}

                    $Result.Session.Name | Should be $TestSessionConfigName
-                    $Result.Session.PSVersion | Should be 6.0
+                    $Result.Session.PSVersion | Should BeExactly $expectedPSVersion


.

adityapatwardhan · 2017-11-27T18:50:26Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -566,6 +579,15 @@ namespace PowershellTestConfigNamespace
                    $Result.Session.UseSharedProcess | Should be $UseSharedProcess
                }

+                It "Validate Set-PSSessionConfiguration -PSVersion" {


This test is very similar to the test at L504. Can you use -testcases?

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.

daxian-dbw · 2017-11-28T00:19:06Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -115,6 +115,8 @@ try
                    }

                    $LocalConfigFilePath = CreateTestConfigFile
+
+                    $expectedPSVersion = "$($PSVersionTable.PSVersion.Major)`.$($PSVersionTable.PSVersion.Minor)"


`.

The backtick is not necessary.

daxian-dbw · 2017-11-28T00:19:06Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -470,6 +472,8 @@ namespace PowershellTestConfigNamespace
                        $TestSessionConfigName = "TestRegisterPSSesionConfig"
                        Unregister-PSSessionConfiguration -Name $TestSessionConfigName -Force -NoServiceRestart -ErrorAction SilentlyContinue
                    }
+
+                    $expectedPSVersion = "$($PSVersionTable.PSVersion.Major)`.$($PSVersionTable.PSVersion.Minor)"


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.

daxian-dbw · 2017-11-28T00:19:06Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

+
+                $Session.Name | Should be $TestSessionConfigName
+                $Session.PSVersion | Should Be 5.1
+            }


Please fix the indentation of this it block.

daxian-dbw · 2017-11-28T00:19:06Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -566,6 +579,15 @@ namespace PowershellTestConfigNamespace
                    $Result.Session.UseSharedProcess | Should be $UseSharedProcess
                }

+                It "Validate Set-PSSessionConfiguration -PSVersion" {


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.

daxian-dbw · 2017-11-28T17:38:02Z

@iSazonov Please add the [Feature] tag to the last commit message to verify whether the changes fix the test failure.

daxian-dbw · 2017-11-28T17:52:47Z

I will add the [Feature] tag to the last commit to trigger the full test run.

markekraus · 2017-11-28T18:44:18Z

test/powershell/Modules/Microsoft.PowerShell.Core/PSSessionConfiguration.Tests.ps1

@@ -566,6 +579,15 @@ namespace PowershellTestConfigNamespace
                    $Result.Session.UseSharedProcess | Should be $UseSharedProcess
                }

+                It "Validate Set-PSSessionConfiguration -PSVersion" {
+
+                    Set-PSSessionConfiguration -Name $TestSessionConfigName -PSVerion 5.1


should be -PSVersion (missing an s)

adityapatwardhan · 2017-11-28T19:40:42Z

Restarted job on Travis-CI as one test failed due to intermittent failure.

TravisEz13 · 2017-11-28T19:40:57Z

Filed #5567 for travis-ci test failure

TravisEz13 · 2017-11-29T00:06:35Z

Not taking for GA because this was caused by a change that is only in master branch

Fix PSVersion in PSSessionConfiguration tests

594e17a

iSazonov force-pushed the fix-pssessionconfiguration-tests branch from 89e8a2d to 594e17a Compare Nov 27, 2017

iSazonov mentioned this pull request Nov 27, 2017

Increase version to 6.1 for PSSessionConfiguration tests #5542

Closed

iSazonov assigned adityapatwardhan Nov 27, 2017

markekraus previously requested changes Nov 27, 2017

View changes

adityapatwardhan requested changes Nov 27, 2017

View changes

markekraus mentioned this pull request Nov 27, 2017

Remove hard coded expected version string #5556

Closed

adityapatwardhan mentioned this pull request Nov 28, 2017

Update test to construct the version from $PSVersionTable.PSVersion #5557

Closed

daxian-dbw requested changes Nov 28, 2017

View changes

markekraus mentioned this pull request Nov 28, 2017

Replace HttpListener Response Tests with WebListener #5540

Merged

daxian-dbw approved these changes Nov 28, 2017

View changes

[Feature] Remove backtick. Use BeExactly Fix indentation.

1ef532a

daxian-dbw force-pushed the fix-pssessionconfiguration-tests branch from 2bab0cd to 1ef532a Compare Nov 28, 2017

adityapatwardhan approved these changes Nov 28, 2017

View changes

markekraus suggested changes Nov 28, 2017

View changes

[Feature] Fix typo.

6b9299c

markekraus approved these changes Nov 28, 2017

View changes

adityapatwardhan merged commit 910c5a4 into PowerShell:master Nov 28, 2017

TravisEz13 added this to the 6.1.0-MQ milestone Nov 29, 2017

iSazonov deleted the fix-pssessionconfiguration-tests branch Nov 29, 2017

Sep	OCT	Nov
	29
2021	2022	2023

Fix PSVersion in PSSessionConfiguration tests #5554

Fix PSVersion in PSSessionConfiguration tests #5554

iSazonov commented Nov 27, 2017

markekraus Nov 27, 2017

iSazonov Nov 27, 2017

markekraus Nov 27, 2017

adityapatwardhan Nov 27, 2017

daxian-dbw Nov 28, 2017

daxian-dbw Nov 28, 2017

iSazonov Nov 28, 2017

daxian-dbw Nov 28, 2017

iSazonov Nov 28, 2017

daxian-dbw Nov 28, 2017

iSazonov Nov 28, 2017

daxian-dbw Nov 28, 2017

daxian-dbw commented Nov 28, 2017

daxian-dbw commented Nov 28, 2017

markekraus Nov 28, 2017

adityapatwardhan commented Nov 28, 2017

TravisEz13 commented Nov 28, 2017

TravisEz13 commented Nov 29, 2017

Fix PSVersion in PSSessionConfiguration tests #5554

Fix PSVersion in PSSessionConfiguration tests #5554

Conversation

iSazonov commented Nov 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daxian-dbw commented Nov 28, 2017

daxian-dbw commented Nov 28, 2017

Choose a reason for hiding this comment

adityapatwardhan commented Nov 28, 2017

TravisEz13 commented Nov 28, 2017

TravisEz13 commented Nov 29, 2017