Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
TravisEz13 merged 4 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
jeffbi:move-item-2385Copy head branch name to clipboard
Jun 23, 2017
Merged

Make Move-Item work with its -Include, -Exclude, and -Filter parameters#3878
TravisEz13 merged 4 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
jeffbi:move-item-2385Copy head branch name to clipboard

Conversation

@jeffbi

@jeffbi jeffbi commented May 30, 2017

Copy link
Copy Markdown

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

jeffbi commented May 30, 2017

Copy link
Copy Markdown
Author

@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

iSazonov commented May 30, 2017

Copy link
Copy Markdown
Collaborator

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

@iSazonov iSazonov left a comment

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.

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

@jeffbi

jeffbi commented May 30, 2017

Copy link
Copy Markdown
Author

@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

iSazonov commented Jun 6, 2017

Copy link
Copy Markdown
Collaborator

@jeffbi Thanks for the fix!

LGTM. (After removing one unneeded comment)

@jeffbi

jeffbi commented Jun 6, 2017

Copy link
Copy Markdown
Author

@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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.