8000 Add `Get-ChildItem` test by iSazonov · Pull Request #10507 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@iSazonov
Copy link
Collaborator
@iSazonov iSazonov commented Sep 10, 2019

PR Summary

Add new test for #3724 to exclude a regression.

PR Context

PR Checklist

@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Sep 10, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.4 milestone Sep 10, 2019
@iSazonov
Copy link
Collaborator Author

/cc @mklement0 for information.

@adityapatwardhan adityapatwardhan changed the title Add Get-ChildItem test Add Get-ChildIte test Sep 11, 2019
@adityapatwardhan adityapatwardhan changed the title Add Get-ChildIte test Add Get-ChildItem test Sep 11, 2019
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 11, 2019
try {
New-Item -Type File 'a`[b]' -ErrorAction SilentlyContinue > $null
$WithInclude = Get-ChildItem * -Include 'a```[b`]'
$WithPath = Get-ChildItem -Path 'a```[b`]'
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest adding another test where escaped and [ and ] are combined with * or ? - because a bug currently lurks there (you inexplicably need an extra round of escaping then); e.g., Get-Item -Path 'a```[b*' breaks at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mklement0 Thanks!

Done.

@adityapatwardhan I don't use -Casesso as not to add a new Describe.

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 removed the test because it seems it looks like a bug. I hope we will discuss this in new issue.

Copy link
Contributor
@mklement0 mklement0 Sep 12, 2019

Choose a reason for hiding this comment

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

@iSazonov: We have a catch-all issue at #7999 and one regarding invalid patterns getting ignored #6733. Perhaps put the tests in commented out for now, with a link to the issues?

Also note that ] doesn't need escaping, as long as the [ is escaped; so, given that Get-ChildItem -Path 'a```[b`]' works, but Get-ChildItem -Path 'a```[b]' (no escaping of ]) doesn't, that should be considered another bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps put the tests in commented out for now, with a link to the issues?

We could add tests in pending state but I think it will be more useful if it is in related discussions than hidden in tests.

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 12, 2019
@adityapatwardhan adityapatwardhan merged commit d7458c4 into PowerShell:master Oct 4, 2019
@iSazonov iSazonov deleted the add-test-dir-wildcard branch October 4, 2019 02:59
@ghost
Copy link
ghost commented Oct 23, 2019

🎉v7.0.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Test Indicates that a PR should be marked as a test change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0