Skip to content

Conversation

@markekraus
Copy link
Contributor

bump version to 6.1 for Get-PSSessionConfiguration and Register-PSSessionConfiguration tests to prevent false fails (example)

@markekraus markekraus changed the title [Feature] Increase version to 6.1 for PSSessionConfiguration tests Increase version to 6.1 for PSSessionConfiguration tests Nov 25, 2017
@iSazonov
Copy link
Collaborator

Can we do these tests more version independent? Can we get the version from PSVersionTable?

@markekraus
Copy link
Contributor Author

I'm not familiar with these tests or the code they are testing, but, one fear I have about doing that kind of thing is if it is somehow referencing itself. if a corrupt version number is leaking into the PSSession, then the test will pass, even though the expected version is wrong. In that case, we should just remove the test as it wont provide any benefit. That is how we get tests that are passing even though the code is broken.

@iSazonov
Copy link
Collaborator

I think the meaning of this test is that this property has a version the same as the current PowerShell Core version.

@markekraus
Copy link
Contributor Author

markekraus commented Nov 25, 2017

Is there a point to that test? What does it test? what is it verifying? why is it important? Maybe it's better to remove this check. if it is just going to echo back the same version every time, is there any value in testing it? Is there any scenario where this would not have the same version? Is this version information significant in any way?

Tests are one area where I think it is OK have these kinds of literal tests. They provide more valuable information that way. If the same version just gets echoed back, testing against that same version it is no different than $PSVersionTable.PSVersion | should be $PSVersionTable.PSVersion

@iSazonov
Copy link
Collaborator

Register-PSSessionConfiguration and Set-PSSessionConfiguration can have -PSVersion. So currently we check that PSVersion is by default current PowerShell version. Also we should add tests that explicit -PSVersion works too.

@markekraus
Copy link
Contributor Author

@iSazonov so, does this check even matter then?

also, please open an issue for the missing tests.

@iSazonov
Copy link
Collaborator

iSazonov commented Nov 27, 2017

I open PR #5554 to fix CIs and add absent tests.

@markekraus markekraus closed this Nov 27, 2017
@markekraus markekraus deleted the PSSessionConfigurationVerBump branch January 19, 2018 18:59
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.

2 participants