Skip to content

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

Merged
scoder merged 2 commits intopython:masterfrom
GPHemsley:improve-xml-etree-tests
Apr 28, 2019
Merged

bpo-32424: Improve test coverage for xml.etree.ElementTree#12891
scoder merged 2 commits intopython:masterfrom
GPHemsley:improve-xml-etree-tests

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

bedevere-bot commented Apr 21, 2019

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

GPHemsley commented Apr 27, 2019

I have made the requested changes; please review again.

@bedevere-bot
Copy link

bedevere-bot commented Apr 27, 2019

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

GPHemsley commented Apr 28, 2019

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

Comments