-
-
Notifications
You must be signed in to change notification settings - Fork 32k
Increase test coverage for pstats.py #57879
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
Comments
This patch increases test coverage for pstats.py from 25 to 36%. It's my first proposed patch so sorry in advance if there are problems. Much more can be done for pstats.py (which is also not much commented) but I want to get some feedback on this first.. |
I don't understand this comment: Also, please don't use docstrings for the test methods because of unittest's "feature" to display them instead of the test method names. |
It's really hard to understand true, and if should not go in the patch in general of course. The sense was that the only test I added is trivial, but I haven't produced something better yet. And ok I will remove the docstrings, I was actually doing it on purpose thinking about the unittest feature, but if the name is clear enough than is better to leave the docstring out... Another thing, I didn't want to use tempfile.mktemp to generate a temporary file, but dump_stats doesn't accept anything else, is it in general safe to use it in this case? |
It has been a long time but if it's still useful sure. I can see some tests have been added in commit 863b1e4 |
The patch needs to be reviewed. If the tests are still relevant and increase coverage, it needs to be converted to a GitHub PR. Otherwise this issue can be closed. |
Now we can close the issue. @tigercosmos @gaogaotiantian |
Uh oh!
There was an error while loading. Please reload this page.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: