-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
gh-78722: Fixed, pathlib.Path.iterdir now throws an exception when path is not valid #8999
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.
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
Conversation
Hi wim glenn,
Thanks for your interest. I'm a new contributor to python. Can you tell me
what is meant by test coverage. Please provide me any existing pull request
which has test coverage.
Thanks
Prudvi RajKumar M
…On Wed, Aug 29, 2018 at 10:58 PM wim glenn ***@***.***> wrote:
Please add test coverage.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8999 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQpj09ZuZLCbmUX8H0uy0xZ09KggOA47ks5uVs87gaJpZM4WR9rH>
.
|
Hi @prudvinit, thanks for opening a PR. I'm not sure that your fix is correct but "test coverage" means writing a test to show code that was broken by the previous behavior of the function you changed so The tests for the pathlib module are in test_pathlib.py, you can look at the other tests of |
Hi
I'd like to continue working on that. I'll add whatever is required and
reach out to you if I need any help.
Thanks
Prudvi M
…On Fri, May 24, 2019, 8:32 PM Rémi Lapeyre ***@***.***> wrote:
Hi @prudvinit <https://github.com/prudvinit>, thanks for opening a PR.
I'm not sure that your fix is correct but "test coverage" means writing a
test to show code that was broken by the previous behavior of the function
you changed so iterdir() in your case, and that your changed fixed. This
serves multiple purposes, it shows the bug, shows your solution work and
the test will fail if someone make another change that ends up breaking
your fix.
The tests for the pathlib module are in test_pathlib.py, you can look at
the other tests of iterdir() to try and write your own. I would be happy
to help you if you get stuck.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8999?email_source=notifications&email_token=AEFGHU27DOCT2C7DLXMV6ZDPW77QHA5CNFSM4FSH3LD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWFUUMQ#issuecomment-495667762>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEFGHU5STUK6MOWHEYLXZXTPW77QHANCNFSM4FSH3LDQ>
.
|
@prudvinit are you still working on this? Otherwise, could we close the PR so that someone else may pick it up? |
Hi Katriel.
Haven't worked on it for a while. I'll work on it during the weekend and
try to finish it. I'll keep you updated,
Thanks
Prudvi M
…On Fri, May 21, 2021 at 7:42 PM Irit Katriel ***@***.***> wrote:
@prudvinit <https://github.com/prudvinit> are you still working on this?
Otherwise, could we close the PR so that someone else may pick it up?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8999 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEFGHU5FMQ4XF43KQHQSOS3TOZS5PANCNFSM4FSH3LDQ>
.
|
Hey @prudvinit - do you think you might have time to resolve conflicts and add tests? This looks like a useful fix! |
Thanks for this PR @prudvinit! In the discussion on the issue, we reached the conclusion that changing behaviour would be too disruptive, and that |
Uh oh!
There was an error while loading. Please reload this page.