admin/PowerShell-PowerShell
1
0
Fork 0
mirror of https://github.com/PowerShell/PowerShell synced 2026-06-08 12:12:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

[Feature] Address Feedback: Use better error message and add more tests

Browse Source
This commit is contained in:
Dongbo Wang
2018-07-10 02:14:59 -07:00
parent c7c35bdcf3
commit 2c9519ba3b
7 changed files with 142 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs
+1 -1
View File
@@ -1249,7 +1249,7 @@ namespace System.Management.Automation.Runspaces
yield return new FormatViewDefinition("ExperimentalFeature",
TableControl.Create()
.AddHeader(Alignment.Left, width: 35)
.AddHeader(Alignment.Right, width: 10)
.AddHeader(Alignment.Right, width: 7)
.AddHeader(Alignment.Left, width: 35)
.AddHeader(Alignment.Left)
.StartRowDefinition()
src/System.Management.Automation/engine/Attributes.cs
+1 -10
View File
@@ -625,16 +625,7 @@ namespace System.Management.Automation
/// </summary>
public ParameterAttribute(string experimentName, ExperimentAction experimentAction)
{
if (string.IsNullOrEmpty(experimentName))
{
throw PSTraceSource.NewArgumentException(nameof(experimentName));
}
if (experimentAction == ExperimentAction.None)
{
throw PSTraceSource.NewArgumentException(nameof(experimentAction));
}
ExperimentalAttribute.ValidateArguments(experimentName, experimentAction);
ExperimentName = experimentName;
ExperimentAction = experimentAction;
}
src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs
+41 -19
View File
@@ -2,8 +2,8 @@
// Licensed under the MIT License.
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Immutable;
using System.Collections.ObjectModel;
using System.Linq;
using System.Management.Automation.Configuration;
using System.Management.Automation.Internal;
@@ -180,16 +180,27 @@ namespace System.Management.Automation
/// <param name="moduleName">When specified, we check if the feature name matches the module name</param>
internal static bool IsModuleFeatureName(string featureName, string moduleName = null)
{
int firstDotIndex = featureName.IndexOf('.');
int lastDotIndex = featureName.LastIndexOf('.');
bool legit = firstDotIndex > 0 && lastDotIndex < featureName.Length - 1;
if (legit && moduleName != null)
// Feature names cannot start with a dot
if (featureName.StartsWith('.'))
{
var moduleNamePart = featureName.AsSpan(0, lastDotIndex);
return moduleNamePart.Equals(moduleName.AsSpan(), StringComparison.OrdinalIgnoreCase);
return false;
}
return legit;
// Feature names must contain a dot, but not at the end
int lastDotIndex = featureName.LastIndexOf('.');
if (lastDotIndex == -1 || lastDotIndex == featureName.Length - 1)
{
return false;
}
if (moduleName == null)
{
return true;
}
// If the module name is given, it must match the prefix of the feature name (up to the last dot).
var moduleNamePart = featureName.AsSpan(0, lastDotIndex);
return moduleNamePart.Equals(moduleName.AsSpan(), StringComparison.OrdinalIgnoreCase);
}
/// <summary>
@@ -266,16 +277,7 @@ namespace System.Management.Automation
/// </summary>
public ExperimentalAttribute(string experimentName, ExperimentAction experimentAction)
{
if (string.IsNullOrEmpty(experimentName))
{
throw PSTraceSource.NewArgumentException(nameof(experimentName));
}
if (experimentAction == ExperimentAction.None)
{
throw PSTraceSource.NewArgumentException(nameof(experimentAction));
}
ValidateArguments(experimentName, experimentAction);
ExperimentName = experimentName;
ExperimentAction = experimentAction;
}
@@ -290,6 +292,26 @@ namespace System.Management.Automation
/// </summary>
internal static readonly ExperimentalAttribute None = new ExperimentalAttribute();
/// <summary>
/// Validate arguments for the constructor.
/// </summary>
internal static void ValidateArguments(string experimentName, ExperimentAction experimentAction)
{
if (string.IsNullOrEmpty(experimentName))
{
string paramName = nameof(experimentName);
throw PSTraceSource.NewArgumentNullException(paramName, Metadata.ArgumentNullOrEmpty, paramName);
}
if (experimentAction == ExperimentAction.None)
{
string paramName = nameof(experimentAction);
string invalidMember = ExperimentAction.None.ToString();
string validMembers = StringUtil.Format("{0}, {1}", ExperimentAction.Hide, ExperimentAction.Show);
throw PSTraceSource.NewArgumentException(paramName, Metadata.InvalidEnumArgument, invalidMember, paramName, validMembers);
}
}
internal bool ToHide => EffectiveAction == ExperimentAction.Hide;
internal bool ToShow => EffectiveAction == ExperimentAction.Show;
src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs
+1 -1
View File
@@ -2083,7 +2083,7 @@ namespace Microsoft.PowerShell.Commands
{
if (writingErrors)
{
string invalidNameStr = String.Join(',', invalidNames);
string invalidNameStr = String.Join(", ", invalidNames);
string errorMsg = StringUtil.Format(Modules.InvalidExperimentalFeatureName, invalidNameStr);
WriteError(new ErrorRecord(new ArgumentException(errorMsg),
"Modules_InvalidExperimentalFeatureName",
src/System.Management.Automation/resources/Metadata.resx
+6
View File
@@ -243,4 +243,10 @@
<data name="ValidateDrivePathNoRoot" xml:space="preserve">
<value>The path argument has no root drive. Supply a full path argument with a root drive.</value>
</data>
<data name="ArgumentNullOrEmpty" xml:space="preserve">
<value>The argument value for the parameter '{0}' cannot be null or an empty string.</value>
</data>
<data name="InvalidEnumArgument" xml:space="preserve">
<value>The Enum member '{0}' is not a valid value for the parameter '{1}'. Specify one of the following members and try again: {2}.</value>
</data>
</root>
test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1
+91 -34
View File
@@ -11,6 +11,8 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
$originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
$PSDefaultParameterValues["it:skip"] = $true
} else {
## Common parameters are defined in the type 'CommonParameters' as public properties.
$CommonParameterCount = [System.Management.Automation.Internal.CommonParameters].GetProperties().Length
$TestModule = Join-Path $PSScriptRoot "assets" "ExpTest"
$AssemblyPath = Join-Path $TestModule "ExpTest.dll"
if (-not (Test-Path $AssemblyPath)) {
@@ -62,8 +64,8 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-Name'
$command.Parameters.Count | Should -Be 12
## Common parameters + '-Name'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 1)
& $Name -Name Joe | Should -BeExactly "Hello World Joe."
}
@@ -75,8 +77,8 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-VMName', '-Port', '-ThrottleLimit' and '-Command'
$command.Parameters.Count | Should -Be 18
## Common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-VMName', '-Port', '-ThrottleLimit' and '-Command'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 7)
$command.ParameterSets.Count | Should -Be 2
$command.Parameters["UserName"].ParameterSets.Count | Should -Be 1
@@ -100,13 +102,13 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
$command.Parameters["Command"].ParameterSets.Count | Should -Be 1
$command.Parameters["Command"].ParameterSets.ContainsKey("__AllParameterSets") | Should -Be $true
## 11 common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-ThrottleLimit' and '-Command'
## Common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-ThrottleLimit' and '-Command'
$command.ParameterSets[0].Name | Should -BeExactly "ComputerSet"
$command.ParameterSets[0].Parameters.Count | Should -Be 16
$command.ParameterSets[0].Parameters.Count | Should -Be ($CommonParameterCount + 5)
## 11 common parameters + '-VMName', '-Port', '-ThrottleLimit' and '-Command'
## Common parameters + '-VMName', '-Port', '-ThrottleLimit' and '-Command'
$command.ParameterSets[1].Name | Should -BeExactly "VMSet"
$command.ParameterSets[1].Parameters.Count | Should -Be 15
$command.ParameterSets[1].Parameters.Count | Should -Be ($CommonParameterCount + 4)
& $Name -UserName "user" -ComputerName "localhost" -ConfigurationName "config" | Should -BeExactly "Invoke-MyCommand with ComputerSet"
& $Name -VMName "VM" -Port "80" | Should -BeExactly "Invoke-MyCommand with VMSet"
@@ -119,8 +121,8 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-SessionName'
$command.Parameters.Count | Should -Be 12
## Common parameters + '-SessionName'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 1)
$command.Parameters["SessionName"].ParameterType.FullName | Should -BeExactly "System.String"
$command.Parameters.ContainsKey("ComputerName") | Should -Be $false
}
@@ -132,8 +134,8 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-ByUrl', '-ByRadio', '-FileName', '-Configuration'
$command.Parameters.Count | Should -Be 15
## Common parameters + '-ByUrl', '-ByRadio', '-FileName', '-Configuration'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 4)
$command.ParameterSets.Count | Should -Be 2
$command.Parameters["ByUrl"].ParameterSets.Count | Should -Be 1
@@ -161,13 +163,13 @@ Describe "Experimental Feature Basic Tests - Feature-Disabled" -tags "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-Name' (dynamic parameters are not triggered)
$command.Parameters.Count | Should -Be 12
## Common parameters + '-Name' (dynamic parameters are not triggered)
$command.Parameters.Count | Should -Be ($CommonParameterCount + 1)
$command.Parameters["Name"] | Should -Not -BeNullOrEmpty
$command = Get-Command $Name -ArgumentList "Joe"
## 11 common parameters + '-Name' and '-ConfigName' (dynamic parameters are triggered)
$command.Parameters.Count | Should -Be 13
## Common parameters + '-Name' and '-ConfigName' (dynamic parameters are triggered)
$command.Parameters.Count | Should -Be ($CommonParameterCount + 2)
$command.Parameters["ConfigName"].Attributes.Count | Should -Be 2
$command.Parameters["ConfigName"].Attributes[0] | Should -BeOfType [parameter]
$command.Parameters["ConfigName"].Attributes[1] | Should -BeOfType [ValidateNotNullOrEmpty]
@@ -186,10 +188,13 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
$originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
$PSDefaultParameterValues["it:skip"] = $true
} else {
## Common parameters are defined in the type 'CommonParameters' as public properties.
$CommonParameterCount = [System.Management.Automation.Internal.CommonParameters].GetProperties().Length
$TestModule = Join-Path $PSScriptRoot "assets" "ExpTest"
$AssemblyPath = Join-Path $TestModule "ExpTest.dll"
if (-not (Test-Path $AssemblyPath)) {
$SourcePath = Join-Path $TestModule "ExpTest.cs"
$SourcePath = (Copy-Item $SourcePath TestDrive:\ -PassThru).FullName
Add-Type -Path $SourcePath -OutputType Library -OutputAssembly $AssemblyPath
}
$moduleInfo = Import-Module $TestModule -PassThru
@@ -234,8 +239,8 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-Name' + '-SwitchOne' + '-SwitchTwo'
$command.Parameters.Count | Should -Be 14
## Common parameters + '-Name' + '-SwitchOne' + '-SwitchTwo'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 3)
$command.ParameterSets.Count | Should -Be 3
& $Name -Name Joe | Should -BeExactly "Hello World Joe."
@@ -251,9 +256,9 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-VMName', '-Port',
## Common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-VMName', '-Port',
## '-Token', '-WebSocketUrl', '-ThrottleLimit' and '-Command'
$command.Parameters.Count | Should -Be 20
$command.Parameters.Count | Should -Be ($CommonParameterCount + 9)
$command.ParameterSets.Count | Should -Be 3
$command.Parameters["UserName"].ParameterSets.Count | Should -Be 1
@@ -285,17 +290,17 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
$command.Parameters["Command"].ParameterSets.Count | Should -Be 1
$command.Parameters["Command"].ParameterSets.ContainsKey("__AllParameterSets") | Should -Be $true
## 11 common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-ThrottleLimit' and '-Command'
## Common parameters + '-UserName', '-ComputerName', '-ConfigurationName', '-ThrottleLimit' and '-Command'
$command.ParameterSets[0].Name | Should -BeExactly "ComputerSet"
$command.ParameterSets[0].Parameters.Count | Should -Be 16
$command.ParameterSets[0].Parameters.Count | Should -Be ($CommonParameterCount + 5)
## 11 common parameters + '-VMName', '-Port', '-ThrottleLimit' and '-Command'
## Common parameters + '-VMName', '-Port', '-ThrottleLimit' and '-Command'
$command.ParameterSets[1].Name | Should -BeExactly "VMSet"
$command.ParameterSets[1].Parameters.Count | Should -Be 15
$command.ParameterSets[1].Parameters.Count | Should -Be ($CommonParameterCount + 4)
## 11 common parameters + '-Token', '-WebSocketUrl', '-ConfigurationName', '-Port', '-ThrottleLimit', '-Command'
## Common parameters + '-Token', '-WebSocketUrl', '-ConfigurationName', '-Port', '-ThrottleLimit', '-Command'
$command.ParameterSets[2].Name | Should -BeExactly "WebSocketSet"
$command.ParameterSets[2].Parameters.Count | Should -Be 17
$command.ParameterSets[2].Parameters.Count | Should -Be ($CommonParameterCount + 6)
& $Name -UserName "user" -ComputerName "localhost" | Should -BeExactly "Invoke-MyCommand with ComputerSet"
& $Name -UserName "user" -ComputerName "localhost" -ConfigurationName "config" | Should -BeExactly "Invoke-MyCommand with ComputerSet"
@@ -314,8 +319,8 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-ComputerName'
$command.Parameters.Count | Should -Be 12
## Common parameters + '-ComputerName'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 1)
$command.Parameters["ComputerName"].ParameterType.FullName | Should -BeExactly "System.String"
$command.Parameters.ContainsKey("SessionName") | Should -Be $false
}
@@ -327,8 +332,8 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
param($Name, $CommandType)
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-ByUrl', '-ByRadio', '-FileName', '-Destination'
$command.Parameters.Count | Should -Be 15
## Common parameters + '-ByUrl', '-ByRadio', '-FileName', '-Destination'
$command.Parameters.Count | Should -Be ($CommonParameterCount + 4)
$command.ParameterSets.Count | Should -Be 2
$command.Parameters["ByUrl"].ParameterSets.Count | Should -Be 1
@@ -356,13 +361,13 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
$command = Get-Command $Name
$command.CommandType | Should -Be $CommandType
## 11 common parameters + '-Name' (dynamic parameters are not triggered)
$command.Parameters.Count | Should -Be 12
## Common parameters + '-Name' (dynamic parameters are not triggered)
$command.Parameters.Count | Should -Be ($CommonParameterCount + 1)
$command.Parameters["Name"] | Should -Not -BeNullOrEmpty
$command = Get-Command $Name -ArgumentList "Joe"
## 11 common parameters + '-Name' and '-ConfigFile' (dynamic parameters are triggered)
$command.Parameters.Count | Should -Be 13
## Common parameters + '-Name' and '-ConfigFile' (dynamic parameters are triggered)
$command.Parameters.Count | Should -Be ($CommonParameterCount + 2)
$command.Parameters["ConfigFile"].Attributes.Count | Should -Be 2
$command.Parameters["ConfigFile"].Attributes[0] | Should -BeOfType [parameter]
$command.Parameters["ConfigFile"].Attributes[1] | Should -BeOfType [ValidateNotNullOrEmpty]
@@ -370,3 +375,55 @@ Describe "Experimental Feature Basic Tests - Feature-Enabled" -Tag "CI" {
$command.Parameters.ContainsKey("ConfigName") | Should -Be $false
}
}
Describe "Expected errors" -Tag "CI" {
It "'[Experimental()]' should fail to construct the attribute" {
{ [Experimental()]param() } | Should -Throw -ErrorId "MethodCountCouldNotFindBest"
}
It "Argument validation for constructors of 'ExperimentalAttribute' and 'ParameterAttribute'" {
{ [Experimental]::new("", "None") } | Should -Throw -ErrorId "PSArgumentNullException"
{ [Experimental]::new([NullString]::Value, "None") } | Should -Throw -ErrorId "PSArgumentNullException"
{ [Experimental]::new("feature", "None") } | Should -Throw -ErrorId "PSArgumentException"
{ [Experimental]::new("feature", "Show") } | Should -Not -Throw
{ [Experimental]::new("feature", "Hide") } | Should -Not -Throw
{ [Parameter]::new("", "None") } | Should -Throw -ErrorId "PSArgumentNullException"
{ [Parameter]::new([NullString]::Value, "None") } | Should -Throw -ErrorId "PSArgumentNullException"
{ [Parameter]::new("feature", "None") } | Should -Throw -ErrorId "PSArgumentException"
{ [Parameter]::new("feature", "Show") } | Should -Not -Throw
{ [Parameter]::new("feature", "Hide") } | Should -Not -Throw
}
It "Feature name check" {
$psd1Content = @'
@{
ModuleVersion = '0.0.1'
CompatiblePSEditions = @('Core')
GUID = 'ce31259c-1804-4016-bc29-083bd2599e19'
PrivateData = @{
PSData = @{
ExperimentalFeatures = @(
@{ Name = '.Feature1'; Description = "Test feature number 1." }
@{ Name = 'Feature2.'; Description = "Test feature number 2." }
@{ Name = 'Feature3'; Description = "Test feature number 3." }
@{ Name = 'Module.Feature4'; Description = "Test feature number 4." }
@{ Name = 'InvalidFeatureName.Feature5'; Description = "Test feature number 5." }
)
}
}
}
'@
$moduleFile = Join-Path $TestDrive InvalidFeatureName.psd1
Set-Content -Path $moduleFile -Value $psd1Content -Encoding Ascii
Import-Module $moduleFile -ErrorVariable featureNameError -ErrorAction SilentlyContinue
$featureNameError | Should -Not -BeNullOrEmpty
$featureNameError[0].FullyQualifiedErrorId | Should -Be "Modules_InvalidExperimentalFeatureName,Microsoft.PowerShell.Commands.ImportModuleCommand"
$featureNameError[0].Exception.Message.Contains(".Feature1") | Should -Be $true
$featureNameError[0].Exception.Message.Contains("Feature2.") | Should -Be $true
$featureNameError[0].Exception.Message.Contains("Feature3") | Should -Be $true
$featureNameError[0].Exception.Message.Contains("Module.Feature4") | Should -Be $true
$featureNameError[0].Exception.Message.Contains("InvalidFeatureName.Feature5") | Should -Be $false
}
}
test/powershell/engine/ExperimentalFeature/assets/ExpTest/ExpTest.psm1
+1 -1
View File
@@ -54,7 +54,7 @@ function Get-GreetingMessage
$message = "Hello World $Name."
if ([ExperimentalFeature]::HasEnabled("ExpTest.FeatureOne"))
if ([ExperimentalFeature]::IsEnabled("ExpTest.FeatureOne"))
{
if ($SwitchOne) { $message += "-SwitchOne is on." }
if ($SwitchTwo) { $message += "-SwitchTwo is on." }