Update CI scripts to support running tests for experimental features #7419
Conversation
|
@TravisEz13 @adityapatwardhan @anmenaga Can you please review this update to our CI scripts? |
| if ($Environment.IsWindows) { | ||
| $content = @" | ||
| { | ||
| "Microsoft.PowerShell:ExecutionPolicy":"RemoteSigned", |
There was a problem hiding this comment.
It would be good to have a comment here about why there is 'RemoteSigned' difference between Windows/Non-Windows code paths.
| if ($Unelevate) | ||
| { | ||
| Start-UnelevatedProcess -process $powershell -arguments @('-noprofile', '-c', $Command) | ||
| Start-UnelevatedProcess -process $powershell -arguments ($PSFlags + "-c $Command") |
There was a problem hiding this comment.
I am thinking about a scenario where I manually run Start-PSPester on my machine...
(in default configuration) Start-PSPester will automatically pick up all Pester tests under $Path, including those for Experimental features, which will fail because I didn't specify $ExperimentalFeatureName parameter?
Or the assumption is that ExperimentalFeature tests will do a runtime check and skip themselves if their corresponding ExperimentalFeature is not enabled?
There was a problem hiding this comment.
Or the assumption is that ExperimentalFeature tests will do a runtime check and skip themselves if their corresponding ExperimentalFeature is not enabled?
Yes, tests for an experimental feature should have runtime checks to determine whether the tests should be skipped. See here as an example: https://github.com/PowerShell/PowerShell/blob/master/test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1
|
To ensure that this change to test infra works as expected it would be good to actually have an experimental feature with corresponding tests checked in as part of this. Having a dummy experimental feature in the product for the sake of testing is probably not worth it. Do we have a candidate for an actually useful experimental feature? |
|
@anmenaga There are tests for a dummy experimental feature -- they are part of my initial check-in for the experimental feature work. They are in this folder https://github.com/PowerShell/PowerShell/tree/master/test/powershell/engine/ExperimentalFeature |
| if ($Unelevate) | ||
| { | ||
| Start-UnelevatedProcess -process $powershell -arguments @('-noprofile', '-c', $Command) | ||
| Start-UnelevatedProcess -process $powershell -arguments ($PSFlags + "-c $Command") |
There was a problem hiding this comment.
Earlier it was '-c', $command now it is -c $command. Intentional change?
There was a problem hiding this comment.
Yes, it's an intentional change. If you look at Start-UnelevatedProcess, you will see the arguments parameter is used like this: "$process $arguments". $argument is a string array, so the elements will be joined together with a space character as the delimiter.
|
@anmenaga and @adityapatwardhan Your comments have been addressed. Please take another look when you can. Thanks! |
3 participants
PR Summary
test/tools/TestMetadata.jsonfor declaring tests for experimental features.-ExperimentalFeatureNametoStart-PSPesterto support running tests with the specified feature turned on.AppVeyor.psm1andtravis.ps1to readTestMetadata.jsonand run tests for each declared features.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests