Skip to content

Make Move-Item work with its -Include, -Exclude, and -Filter parameters#3878

Merged
TravisEz13 merged 4 commits intoPowerShell:masterfrom
jeffbi:move-item-2385
Jun 23, 2017
Merged

Make Move-Item work with its -Include, -Exclude, and -Filter parameters#3878
TravisEz13 merged 4 commits intoPowerShell:masterfrom
jeffbi:move-item-2385

Conversation

@jeffbi
Copy link
Copy Markdown

@jeffbi jeffbi commented May 30, 2017

Fixed #2385

Invoke the correct overload of SessionState.Path.GetResolvedPSPathFromPSPath, passing the cmdlet context object.

…rs (#2385)

Invoke the correct overload of SessionState.Path.GetResolvedPSPathFromPSPath, passing the cmdlet context object.
@jeffbi
Copy link
Copy Markdown
Author

jeffbi commented May 30, 2017

@iSazonov Can you take a look at the Appveyor failure? I'm looking at the details and they seem to show two failure points, neither of which have anything to do with this PR.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented May 30, 2017

@jeffbi I see the same in other PRs. Nightly builds is affected too.

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

Original Issue say that the cmdlet works well but writes unexpected error message - I don't see the check in tests.

@jeffbi
Copy link
Copy Markdown
Author

jeffbi commented May 30, 2017

@iSazonov Well, what it actually said what that the cmdlet successfully performed the action that was intended, then wrote an error. That's not quite the same as it worked well.

I've updated the tests to check that the Move-Item was successful, and also to verify that files meant to be unaffected were unaffected.

@iSazonov
Copy link
Copy Markdown
Collaborator

LGTM.

$booContent = "boo content"
}
BeforeEach {
New-Item -ItemType Directory -Path $filterPath
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be

New-Item -ItemType Directory -Path $filterPath | Out-Null

to avoid on console output of created item

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed.

It "Can move to different directory, filtered with -Include" {
Move-Item -Path $filePath -Destination $moveToPath -Include "bar*"
$? | Should Be $true
Test-Path -Path $barPath | Should Be $false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use Powershell $barPath | Should Not Exist"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

}
It "Can move to different directory, filtered with -Include" {
Move-Item -Path $filePath -Destination $moveToPath -Include "bar*"
$? | Should Be $true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Difficult to debug from logs. Instead use

Move-Item -Path $filePath -Destination $moveToPath -Include "bar*" -ErrorVariable e -ErrorAction SilentlyContinue
$e | Should BeNullOrEmpty

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

It "Can move to different directory, filtered with -Include" {
Move-Item -Path $filePath -Destination $moveToPath -Include "bar*" -ErrorVariable e -ErrorAction SilentlyContinue
$e | Should BeNullOrEmpty
#Test-Path -Path $barPath | Should Be $false
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please remove the comment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for catching that. Fixed.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Jun 6, 2017

@jeffbi Thanks for the fix!

LGTM. (After removing one unneeded comment)

@jeffbi
Copy link
Copy Markdown
Author

jeffbi commented Jun 6, 2017

@iSazonov, @adityapatwardhan Thanks for the review.

@TravisEz13
Copy link
Copy Markdown
Member

@jeffbi Can you update or remove the company in your profile? If it is accurate, please email me internally about additional steps you need to take.

@adityapatwardhan Please make your Microsoft Organization membership public.

@TravisEz13 TravisEz13 merged commit 5ee4ec1 into PowerShell:master Jun 23, 2017
mirichmo added a commit that referenced this pull request Jun 23, 2017
@jeffbi jeffbi deleted the move-item-2385 branch July 11, 2017 19:24
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.

5 participants