Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test-Path: Return $false when given an empty or $null -Path/-LiteralPath value #8080

Merged
merged 15 commits into from
Nov 21, 2018
Merged

Test-Path: Return $false when given an empty or $null -Path/-LiteralPath value #8080

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
1313095
Add empty / null path handling to Test-Path
vexx32 Oct 19, 2018
7d5735a
Update tests to fail correctly under the sole remaining erroring cond…
vexx32 Oct 19, 2018
2f41783
Use quick-exit code paths instead of lengthy if/else
vexx32 Oct 19, 2018
44417f5
Add non-terminating errors for null input
vexx32 Oct 25, 2018
63e1b8f
Reformat test file for readability
vexx32 Oct 25, 2018
d793658
Use more descriptive error ID
vexx32 Oct 25, 2018
794e78b
Add Tests for new behaviour
vexx32 Oct 25, 2018
cae5294
Remove unnecessary blank line per StyleCop
vexx32 Oct 25, 2018
60810ca
Add new tests
vexx32 Oct 30, 2018
e731fab
Clarify it name
vexx32 Oct 30, 2018
2008e51
Fix endless loop caused by change of error type in tests
vexx32 Oct 30, 2018
28cc91f
Fix FullyQualifiedErrorID
vexx32 Oct 31, 2018
0be7bf1
Consolidate ErrorIDs to match
vexx32 Oct 31, 2018
890fcb1
Add .resx file for TestPath
vexx32 Nov 16, 2018
cddfb79
Use resource file for error string
vexx32 Nov 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/Microsoft.PowerShell.Commands.Management/commands/management/PingPathCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Management.Automation;
using Dbg = System.Management.Automation;

Expand Down Expand Up @@ -41,6 +42,9 @@ public class TestPathCommand : CoreCommandWithCredentialsBase
/// </summary>
[Parameter(Position = 0, ParameterSetName = "Path",
Mandatory = true, ValueFromPipeline = true, ValueFromPipelineByPropertyName = true)]
[AllowNull]
[AllowEmptyCollection]
iSazonov marked this conversation as resolved.
Show resolved Hide resolved
[AllowEmptyString]
public string[] Path
{
get { return _paths; }
Expand All @@ -53,6 +57,9 @@ public string[] Path
[Parameter(ParameterSetName = "LiteralPath",
Mandatory = true, ValueFromPipeline = false, ValueFromPipelineByPropertyName = true)]
[Alias("PSPath", "LP")]
[AllowNull]
[AllowEmptyCollection]
[AllowEmptyString]
public string[] LiteralPath
{
get { return _paths; }
Expand Down Expand Up @@ -124,7 +131,7 @@ internal override object GetDynamicParameters(CmdletProviderContext context)

if (this.PathType == TestPathType.Any && !IsValid)
{
if (Path != null && Path.Length > 0)
if (Path != null && Path.Length > 0 && Path[0] != null)
{
result = InvokeProvider.Item.ItemExistsDynamicParameters(Path[0], context);
}
Expand All @@ -133,6 +140,7 @@ internal override object GetDynamicParameters(CmdletProviderContext context)
result = InvokeProvider.Item.ItemExistsDynamicParameters(".", context);
}
}

return result;
} // GetDynamicParameters

Expand All @@ -154,12 +162,39 @@ internal override object GetDynamicParameters(CmdletProviderContext context)
/// </summary>
protected override void ProcessRecord()
{
if (_paths == null || _paths.Length == 0)
{
WriteError(new ErrorRecord(
new ArgumentNullException("The path was null or an empty collection."),
vexx32 marked this conversation as resolved.
Show resolved Hide resolved
"NullPathNotPermitted",
ErrorCategory.InvalidArgument,
Path));

return;
}

CmdletProviderContext currentContext = CmdletProviderContext;

foreach (string path in _paths)
{
bool result = false;

if (path == null)
{
WriteError(new ErrorRecord(
new ArgumentNullException("The path was null or an empty collection."),
vexx32 marked this conversation as resolved.
Show resolved Hide resolved
"NullPath",
ErrorCategory.InvalidArgument,
Path));
continue;
}

if (string.IsNullOrWhiteSpace(path))
{
WriteObject(result);
continue;
}

try
{
if (IsValid)
Expand All @@ -184,6 +219,7 @@ protected override void ProcessRecord()
}
}
}

// Any of the known exceptions means the path does not exist.
catch (PSNotSupportedException)
{
Expand Down
8 changes: 4 additions & 4 deletions test/powershell/Language/Scripting/ErrorPosition.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ for ($null[0];
'@
$for_pipeline_initializer_script = @'
$test = 1
for (Test-Path $null;
for (Test-Path $null, $null;
$test -gt 1;) { }
'@
$for_expression_condition_script = @'
Expand All @@ -27,7 +27,7 @@ for (;$null[0];)
'@
$for_pipeline_condition_script = @'
$test = 1
for (;Test-Path $null;)
for (;Test-Path $null, $null;)
{ }
'@
$do_while_expression_condition_script = @'
Expand All @@ -38,7 +38,7 @@ while ($null[0])
$do_while_pipeline_condition_script = @'
$test = 1
do {}
while (Test-Path $null)
while (Test-Path $null, $null)
'@
$do_until_expression_condition_script = @'
$test = 1
Expand All @@ -48,7 +48,7 @@ until ($null[0])
$do_until_pipeline_condition_script = @'
$test = 1
do {}
until (Test-Path $null)
until (Test-Path $null, $null)
'@

$testCases = @(
Expand Down
101 changes: 57 additions & 44 deletions test/powershell/Modules/Microsoft.PowerShell.Management/Test-Path.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,123 +2,136 @@
# Licensed under the MIT License.
Describe "Test-Path" -Tags "CI" {
BeforeAll {
$testdirectory = $TestDrive
$testfilename = New-Item -path $testdirectory -Name testfile.txt -ItemType file -Value 1 -force
$testdirectory = $TestDrive
$testfilename = New-Item -path $testdirectory -Name testfile.txt -ItemType file -Value 1 -force

# populate with additional files
New-Item -Path $testdirectory -Name datestfile -value 1 -ItemType file | Out-Null
New-Item -Path $testdirectory -Name gatestfile -value 1 -ItemType file | Out-Null
New-Item -Path $testdirectory -Name usr -value 1 -ItemType directory | Out-Null
# populate with additional files
New-Item -Path $testdirectory -Name datestfile -value 1 -ItemType file | Out-Null
New-Item -Path $testdirectory -Name gatestfile -value 1 -ItemType file | Out-Null
New-Item -Path $testdirectory -Name usr -value 1 -ItemType directory | Out-Null

$nonExistentDir = Join-Path -Path (Join-Path -Path $testdirectory -ChildPath usr) -ChildPath bin
$nonExistentPath = Join-Path -Path (Join-Path -Path (Join-Path -Path $testdirectory -ChildPath usr) -ChildPath bin) -ChildPath error
$nonExistentDir = Join-Path -Path (Join-Path -Path $testdirectory -ChildPath usr) -ChildPath bin
$nonExistentPath = Join-Path -Path (Join-Path -Path (Join-Path -Path $testdirectory -ChildPath usr) -ChildPath bin) -ChildPath error
}

It "Should be called on an existing path without error" {
{ Test-Path $testdirectory } | Should -Not -Throw
{ Test-Path -Path $testdirectory } | Should -Not -Throw
{ Test-Path -LiteralPath $testdirectory } | Should -Not -Throw
{ Test-Path $testdirectory } | Should -Not -Throw
{ Test-Path -Path $testdirectory } | Should -Not -Throw
{ Test-Path -LiteralPath $testdirectory } | Should -Not -Throw
}

It "Should allow piping objects to it" {
{ $testdirectory | Test-Path } | Should -Not -Throw
{ $testdirectory | Test-Path } | Should -Not -Throw

$testdirectory | Test-Path | Should -BeTrue
$nonExistentDir | Test-Path | Should -BeFalse
$testdirectory | Test-Path | Should -BeTrue
$nonExistentDir | Test-Path | Should -BeFalse
}

It "Should be called on a nonexistent path without error" {
{ Test-Path -Path $nonExistentPath } | Should -Not -Throw
{ Test-Path -Path $nonExistentPath } | Should -Not -Throw
}

It "Should return false for a nonexistent path" {
Test-Path -Path $nonExistentPath | Should -BeFalse
Test-Path -Path $nonExistentPath | Should -BeFalse
}

It "Should return true for an existing path" {
{ Test-Path -Path $testdirectory } | Should -BeTrue
{ Test-Path -Path $testdirectory } | Should -BeTrue
}

It 'Should return false for an empty string' {
Test-Path -Path '' | Should -BeFalse
}

It 'Should return false for a whitespace string' {
Test-Path -Path ' ' | Should -BeFalse
}

It 'Should write a non-terminating error when given a null path' {
{ Test-Path -Path $null -ErrorAction Stop } | Should -Throw -ErrorId 'NullPathNotPermitted'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should tests here Test-Path $null, $null;.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also had to modify some ErrorPosition tests as they were causing infinite loops when the cmldet simply wrote a non-terminating error.

{ Test-Path -Path $null -ErrorAction SilentlyContinue } | Should -Not -Throw
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems we should remove this. I am not sure that it tests "non-terminating"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I thought, too. Surprisingly, though, -ErrorAction SilentlyContinue will still trigger a | should -throw if and only if the error is terminating to start with (either throw or ThrowTerminatingError())

@indented-automation had to explain that one to me. :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adityapatwardhan @JamesWTruher @SteveL-MSFT Can we use the pattern to test non-terminating errors?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this change is explicitly to be a non-terminating error, this pattern seems fine. But should probably be followed up with -ErrorAction Stop to verify the non-terminating error is correct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow? I don't see a need to double-check it with -ErrorAction Stop

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we need to add a comment to describe the pattern.

}

It "Should be able to accept a regular expression" {
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u*") } | Should -Not -Throw
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u[a-z]r") } | Should -Not -Throw
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u*") } | Should -Not -Throw
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u[a-z]r") } | Should -Not -Throw
}

It "Should be able to return the correct result when a regular expression is used" {
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u*") | Should -BeTrue
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u[a-z]*") | Should -BeTrue
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u*") | Should -BeTrue
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u[a-z]*") | Should -BeTrue

Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "aoeu*") | Should -BeFalse
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u[A-Z]") | Should -BeFalse
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "aoeu*") | Should -BeFalse
Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "u[A-Z]") | Should -BeFalse
}

It "Should return false when the Leaf pathtype is used on a directory" {
Test-Path -Path $testdirectory -PathType Leaf | Should -BeFalse
Test-Path -Path $testdirectory -PathType Leaf | Should -BeFalse
}

It "Should return true when the Leaf pathtype is used on an existing endpoint" {
Test-Path -Path $testfilename -PathType Leaf | Should -BeTrue
Test-Path -Path $testfilename -PathType Leaf | Should -BeTrue
}

It "Should return false when the Leaf pathtype is used on a nonexistent file" {
Test-Path -Path "aoeu" -PathType Leaf | Should -BeFalse
Test-Path -Path "aoeu" -PathType Leaf | Should -BeFalse
}

It "Should return true when the Leaf pathtype is used on a file using the Type alias instead of PathType" {
Test-Path -Path $testfilename -Type Leaf | Should -BeTrue
Test-Path -Path $testfilename -Type Leaf | Should -BeTrue
}

It "Should be able to search multiple regular expressions using the include switch" {
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "*") -Include t* } | Should -BeTrue
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "*") -Include t* } | Should -BeTrue
}

It "Should be able to exclude a regular expression using the exclude switch" {
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "*") -Exclude v* } | Should -BeTrue
{ Test-Path -Path (Join-Path -Path $testdirectory -ChildPath "*") -Exclude v* } | Should -BeTrue
}

It "Should be able to exclude multiple regular expressions using the exclude switch" {
# tests whether there's any files in the usr directory that don't start with 'd' or 'g'
{ Test-Path -Path $testfilename -Exclude d*, g* } | Should -BeTrue
# tests whether there's any files in the usr directory that don't start with 'd' or 'g'
{ Test-Path -Path $testfilename -Exclude d*, g* } | Should -BeTrue
}

It "Should return true if the syntax of the path is correct when using the IsValid switch" {
{ Test-Path -Path $nonExistentPath -IsValid } | Should -BeTrue
{ Test-Path -Path $nonExistentPath -IsValid } | Should -BeTrue
}

It "Should return false if the syntax of the path is incorrect when using the IsValid switch" {
$badPath = " :;!@#$%^&*(){}?+|_-"
Test-Path -Path $badPath -IsValid | Should -BeFalse
$badPath = " :;!@#$%^&*(){}?+|_-"
Test-Path -Path $badPath -IsValid | Should -BeFalse
}

It "Should return true on paths containing spaces when the path is surrounded in quotes" {
Test-Path -Path "/totally a valid/path" -IsValid | Should -BeTrue
Test-Path -Path "/totally a valid/path" -IsValid | Should -BeTrue
}

It "Should throw on paths containing spaces when the path is not surrounded in quotes" {
{ Test-Path -Path /a path/without quotes/around/it -IsValid } | Should -Throw -ErrorId "PositionalParameterNotFound,Microsoft.PowerShell.Commands.TestPathCommand"
{ Test-Path -Path /a path/without quotes/around/it -IsValid } | Should -Throw -ErrorId "PositionalParameterNotFound,Microsoft.PowerShell.Commands.TestPathCommand"
}

It "Should return true if a directory leads or trails with a space when surrounded by quotes" {
Test-Path -Path "/a path / with/funkyspaces" -IsValid | Should -BeTrue
Test-Path -Path "/a path / with/funkyspaces" -IsValid | Should -BeTrue
}

It "Should return true on a valid path when the LiteralPath switch is used" {
Test-Path -LiteralPath $testfilename | Should -BeTrue
Test-Path -LiteralPath $testfilename | Should -BeTrue
}

It "Should return false if regular expressions are used with the LiteralPath switch" {
Test-Path -LiteralPath (Join-Path -Path $testdirectory -ChildPath "u*") | Should -BeFalse
Test-Path -LiteralPath (Join-Path -Path $testdirectory -ChildPath "u[a-z]r") | Should -BeFalse
Test-Path -LiteralPath (Join-Path -Path $testdirectory -ChildPath "u*") | Should -BeFalse
Test-Path -LiteralPath (Join-Path -Path $testdirectory -ChildPath "u[a-z]r") | Should -BeFalse
}

It "Should return false if regular expressions are used with the LiteralPath alias PSPath switch" {
Test-Path -PSPath (Join-Path -Path $testdirectory -ChildPath "u*") | Should -BeFalse
Test-Path -PSPath (Join-Path -Path $testdirectory -ChildPath "u[a-z]r") | Should -BeFalse
Test-Path -PSPath (Join-Path -Path $testdirectory -ChildPath "u*") | Should -BeFalse
Test-Path -PSPath (Join-Path -Path $testdirectory -ChildPath "u[a-z]r") | Should -BeFalse
}

It "Should return true if used on components other than filesystem objects" {
Test-Path Alias:\gci | Should -BeTrue
Test-Path Env:\PATH | Should -BeTrue
Test-Path Alias:\gci | Should -BeTrue
Test-Path Env:\PATH | Should -BeTrue
}

}