Skip to content

Conversation

@karlcow
Copy link
Contributor

@karlcowkarlcow 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-actionsgithub-actionsbot added the stale Stale PR or inactive for long period of time. label Feb 19, 2021
@github-actionsgithub-actionsbot removed the stale Stale PR or inactive for long period of time. label Aug 2, 2022
@terryjreedyterryjreedy changed the title bpo-19683: Adds tests for xml.dom.minidomgh-63882: Adds tests for xml.dom.minidomNov 29, 2022
@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
ContributorAuthor

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 freeto 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

@karlcow@terryjreedy@the-knights-who-say-ni@ezio-melotti@bedevere-bot