Skip to content

bpo-32424: Improve test coverage for xml.etree.ElementTree #12891

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 28, 2019
Merged

bpo-32424: Improve test coverage for xml.etree.ElementTree #12891

merged 2 commits into from
Apr 28, 2019

Conversation

GPHemsley
Copy link
Contributor

@GPHemsley GPHemsley commented Apr 20, 2019 •
edited by bedevere-bot
Loading

(Spun off from #5046.)

Add unit tests to improve coverage for xml.etree.ElementTree.

According to my testing, these should all pass in both the C and Python implementations of ElementTree.

https://bugs.python.org/issue32424

@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Apr 20, 2019
@GPHemsley
Copy link
Contributor Author

GPHemsley commented Apr 20, 2019 •
edited
Loading

@serhiy-storchaka @scoder Can you review these with fresh eyes? I haven't necessarily attempted to address comments from the previous PR, but I want to make sure we at least have a separation of concerns.

scoder
scoder requested changes Apr 21, 2019
View reviewed changes
Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

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

The comments from GH-5046 still apply.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@GPHemsley
Copy link
Contributor Author

GPHemsley commented Apr 21, 2019 •
edited
Loading

The comments from GH-5046 still apply.

I was hoping to get them fresh here to ensure that discussions didn't get conflated. (Note that I removed tests that would fail due to the other issues discussed in #5046 and elsewhere.)

scoder
scoder reviewed Apr 21, 2019
View reviewed changes
Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

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

I added the comments that I think still apply.

@scoder scoder changed the title Improve test coverage for xml.etree.ElementTree bpo-32424: Improve test coverage for xml.etree.ElementTree Apr 21, 2019
@GPHemsley GPHemsley force-pushed the improve-xml-etree-tests branch from fec9b9b to 8f4adcd Compare April 21, 2019 21:36
@GPHemsley
Copy link
Contributor Author

GPHemsley commented Apr 21, 2019 •
edited by bedevere-bot
Loading

@scoder Note that I explicitly did not include an issue number on this PR because it is not directly tied to any issue and does not resolve any open tickets. Rather, it is a prerequisite to my changes for both bpo-32424 and bpo-36685.

@GPHemsley GPHemsley force-pushed the improve-xml-etree-tests branch from 8f4adcd to 96e6599 Compare April 21, 2019 21:55
scoder
scoder reviewed Apr 24, 2019
View reviewed changes
@GPHemsley GPHemsley force-pushed the improve-xml-etree-tests branch from 96e6599 to 7338f47 Compare April 27, 2019 19:21
GPHemsley
GPHemsley commented Apr 27, 2019
View reviewed changes
Copy link
Contributor Author

@GPHemsley GPHemsley left a comment

Choose a reason for hiding this comment

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

Addressed comments.

@GPHemsley
Copy link
Contributor Author

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@scoder: please review the changes made to this pull request.

@scoder
Copy link
Contributor

scoder commented Apr 28, 2019

Thanks.

@scoder scoder merged commit 50fed0b into python:master Apr 28, 2019
@GPHemsley
Copy link
Contributor Author

Thanks for sticking with this!

@GPHemsley GPHemsley mentioned this pull request Apr 28, 2019
Merged
@GPHemsley GPHemsley mannequin mentioned this pull request Jun 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

@scoder scoder scoder approved these changes

Assignees
No one assigned
Labels
tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@GPHemsley @bedevere-bot @scoder @serhiy-storchaka @the-knights-who-say-ni