GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-63882: Implement some `test_minidom` tests by StanFromIreland · Pull Request #132879 · python/cpython

Conversation

@StanFromIreland

@StanFromIreland StanFromIreland commented Apr 24, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Applies: https://bugs.python.org/file33258/issue19683.patch

cc @terryjreedy @hugovk @picnixz

  • Issue: test_minidom has many empty tests #63882

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting review tests Tests in the Lib/test dir labels Apr 24, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 24, 2025
test_minidom has many empty tests #63882
Closed
@picnixz picnixz added the skip news label Apr 24, 2025
@hugovk

hugovk commented Apr 24, 2025

Copy link
Copy Markdown
Member

Please include the original patch author in a Co-authored-by trailer.

https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors
StanFromIreland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland and others added 2 commits April 24, 2025 16:45
@StanFromIreland @JulianGindi
Apply patch
6c5b9b0 
Co-authored-by: Julian Gindi <[email protected]>
@StanFromIreland
Clean up
ec9c382
@StanFromIreland
StanFromIreland force-pushed the apply-patch branch from e93be5b to ec9c382 Compare April 24, 2025 15:46
@StanFromIreland

This comment was marked as resolved.

Sign in to view
hugovk
hugovk reviewed Apr 25, 2025
View reviewed changes
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@StanFromIreland @hugovk
Update Lib/test/test_minidom.py
df9feb8 
Co-authored-by: Hugo van Kemenade <[email protected]>
picnixz
picnixz reviewed Apr 25, 2025
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I've added some comments concerning unlink but AFAIK, this is only used to break internal references within the DOM itself. Since the objects are either discarded at the end of the function or are small, you might also forget about calling .unlink. OTOH, we can be consistent with the existing tests and always call unlink in an addCleanup instead.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland and others added 2 commits April 25, 2025 15:53
@StanFromIreland @picnixz
Apply suggestions from code review
c3b759c 
Co-authored-by: Bénédikt Tran <[email protected]>
@StanFromIreland
Benedikt's suggestions
540a359
@StanFromIreland
StanFromIreland requested review from hugovk and picnixz April 25, 2025 15:05
hugovk
hugovk approved these changes Apr 26, 2025
View reviewed changes

@hugovk hugovk left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Apr 26, 2025
picnixz
picnixz approved these changes Apr 26, 2025
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM but I have a question.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland
StanFromIreland commented Apr 26, 2025
View reviewed changes
Comment thread Lib/test/test_minidom.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@StanFromIreland
Update Lib/test/test_minidom.py
1d6e5eb
@StanFromIreland

StanFromIreland commented Apr 26, 2025

Copy link
Copy Markdown
Member Author

Question has been resolved.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
picnixz
picnixz approved these changes Apr 26, 2025
View reviewed changes
@picnixz picnixz self-assigned this Apr 26, 2025
@picnixz

picnixz commented Apr 26, 2025

Copy link
Copy Markdown
Member

@hugovk I'll actually let you write the commit message as I don't know how you want the original patch's author to be credited.
StanFromIreland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz picnixz removed their assignment Apr 26, 2025
@StanFromIreland StanFromIreland mentioned this pull request Apr 26, 2025
gh-63882: Use self.assert* in test_minidom #133000
Merged
@hugovk
hugovk merged commit ee033d4 into python:main Apr 26, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 26, 2025
@hugovk

hugovk commented Apr 26, 2025

Copy link
Copy Markdown
Member

Thanks, just to make sure the Co-authored-by was in the commit description.
StanFromIreland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk hugovk added the needs backport to 3.13 only security fixes label Apr 26, 2025
@miss-islington-app

miss-islington-app Bot commented Apr 26, 2025

Copy link
Copy Markdown

Thanks @StanFromIreland for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 26, 2025
@StanFromIreland @JulianGindi
@hugovk @picnixz @miss-islington
pythongh-63882: Implement some test_minidom tests (pythonGH-132879)
0496125 
(cherry picked from commit ee033d4)

Co-authored-by: Stan Ulbrych <[email protected]>
Co-authored-by: Julian Gindi <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

bedevere-app Bot commented Apr 26, 2025

Copy link
Copy Markdown

GH-133029 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Apr 26, 2025
@StanFromIreland
StanFromIreland deleted the apply-patch branch April 26, 2025 20:07
@StanFromIreland

StanFromIreland commented Apr 26, 2025

Copy link
Copy Markdown
Member Author

It should have been auto populated by gh I believe?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hugovk added a commit that referenced this pull request Apr 26, 2025
@miss-islington @StanFromIreland
@JulianGindi @hugovk @picnixz
[3.13] gh-63882: Implement some test_minidom tests (GH-132879) (#13…
fdcaaad 
…3029)

Co-authored-by: Stan Ulbrych <[email protected]>
Co-authored-by: Julian Gindi <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-bot

bedevere-bot commented Apr 26, 2025

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.13 (tier-3) has failed when building commit fdcaaad.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1386/builds/1246) and take a look at the build logs.
  4. Check if the failure is related to this commit (fdcaaad) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1386/builds/1246

Summary of the results of the build (if available):

==

Click to see traceback logs 
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/E06CC346-3BDC-41D4-B51D-C70ADBC67E86/data/Containers/Bundle/Application/3439A6D5-E051-44C2-BFBB-1AD159C2D958/iOSTestbed.app/python/lib/python3.13/threading.py", line 1041, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/E06CC346-3BDC-41D4-B51D-C70ADBC67E86/data/Containers/Bundle/Application/3439A6D5-E051-44C2-BFBB-1AD159C2D958/iOSTestbed.app/python/lib/python3.13/threading.py", line 992, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/E06CC346-3BDC-41D4-B51D-C70ADBC67E86/data/Containers/Bundle/Application/3439A6D5-E051-44C2-BFBB-1AD159C2D958/iOSTestbed.app/python/lib/python3.13/test/test_interpreters/test_stress.py", line 47, in run
    interp = interpreters.create()
  File "/Users/buildbot/Library/Developer/XCTestDevices/E06CC346-3BDC-41D4-B51D-C70ADBC67E86/data/Containers/Bundle/Application/3439A6D5-E051-44C2-BFBB-1AD159C2D958/iOSTestbed.app/python/lib/python3.13/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@StanFromIreland

StanFromIreland commented Apr 26, 2025

Copy link
Copy Markdown
Member Author

Unrelated fail.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes
@picnixz picnixz picnixz approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@StanFromIreland @hugovk @picnixz @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.