Skip to content
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

gh-63882: Adds tests for xml.dom.minidom #24152

Closed
wants to merge 21 commits into from
Closed

Conversation

karlcow
Copy link
Contributor

@karlcow karlcow commented Jan 7, 2021

This is a work in progress not yet ready for review.

https://bugs.python.org/issue19683

it also removes the old tests so we get more regular tests
Removes the old tests.
Adds new tests in a regular form and testing more cases
Adds docstring
These tests are useless for now. We will be adding them again later with proper code.
Some parts of the function were not tested. This is fixing it.
* Convert old tests to have a regular form
* Adds new tests to cover more cases.
* Adds tests for removeChild
* Remove old tests
* Fix the docstring for removeChild and replaceChild
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 19, 2021
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 2, 2022
@terryjreedy terryjreedy changed the title bpo-19683: Adds tests for xml.dom.minidom gh-63882: Adds tests for xml.dom.minidom Nov 29, 2022
@terryjreedy terryjreedy requested a review from zware November 29, 2022 01:18
@terryjreedy
Copy link
Member

terryjreedy commented Nov 29, 2022

The merge conflict is a result of new imports for pyexpat and parsers...ExpatError in main. I could easily fix the conflict, but I really do not like putting multiple imports from one module in separate lines and would undo an revise that change also at the same time.

@terryjreedy
Copy link
Member

I consider this unreviewable until split into multiple PRs. See #63882 (comment)

@karlcow
Copy link
Contributor Author

karlcow commented Nov 29, 2022

I can't work on this anymore. Unfortunately.

@terryjreedy
Copy link
Member

@karlcow Thank you for this draft. The docstring additions have been moved to a new issue #128508 and PR #128477 for revisions. You are listed on the latter as a co-author.

The empty tests should be commented out until replaced, not removed. A new PR should close the issue.

The revised tests, fixing names and code, should be moved to another new issue. With this is done, the tests for class methods should be put in separate TestCase classes. MinidomTest should only have the module function tests.

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

Successfully merging this pull request may close these issues.

5 participants