gh-128816: fix an import warning in test_doctest.py when importing re… by graingert · Pull Request #128817 · python/cpython

graingert · 2025-01-14T09:05:00Z

…adline in WASI

Issue: test_doctests failure when running tests with -werror on platforms without readline (win32, wasi) #128816

…ing readline in WASI

serhiy-storchaka

LGTM.

But it seems that these methods are never called (only checked for existence). I would raise an AssertionError to ensure that they are not called.

graingert · 2025-01-14T11:57:37Z

I believe they are called, eg when attempting to import readline and it is missing. I will check

graingert · 2025-01-14T14:28:36Z

LGTM.

But it seems that these methods are never called (only checked for existence). I would raise an AssertionError to ensure that they are not called.

@serhiy-storchaka I checked and both functions are called, not just checked for existence

serhiy-storchaka

Oh, I missed that it is only reproducible if readline is not available. That is why I was not able to reproduce this issue.

LGTM.

graingert · 2025-01-15T08:10:01Z

thanks for the review, but I can't merge it

serhiy-storchaka

On other hand, the test now emit a different warning (even if readline is available):

test_lineendings (test.test_doctest.test_doctest)
Doctest: test.test_doctest.test_doctest.test_lineendings ... .../Lib/test/test_doctest/test_doctest.py:2887: DeprecationWarning: importlib.abc.ResourceLoader is deprecated in favour of supporting resource loading through importlib.resources.abc.ResourceReader.
  self.importer = TestImporter()
ok

And it fails with -Werror.

So this is not a complete fix.

graingert · 2025-01-15T10:41:02Z

@serhiy-storchaka @brettcannon see #128862

This reverts commit 9b850e7.

graingert · 2025-01-15T10:46:00Z

I have made the requested changes; please review again

bedevere-app · 2025-01-15T10:46:05Z

Thanks for making the requested changes!

@serhiy-storchaka: please review the changes made to this pull request.

serhiy-storchaka

LGTM! 👍

miss-islington-app · 2025-01-15T13:06:03Z

Thanks @graingert for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington-app · 2025-01-15T13:38:19Z

Thanks @graingert for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2025-01-15T13:38:19Z

Thanks @graingert for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

* Fix a deprecation warning for using importlib.resources.abc.ResourceReader. * Fix an import warning when importing readline (if it has not yet been imported). (cherry picked from commit 599be68) Co-authored-by: Thomas Grainger <tagrain@gmail.com>

bedevere-app · 2025-01-15T13:38:34Z

GH-128870 is a backport of this pull request to the 3.13 branch.

bedevere-app · 2025-01-15T13:38:39Z

GH-128871 is a backport of this pull request to the 3.12 branch.

* Fix a deprecation warning for using importlib.resources.abc.ResourceReader. * Fix an import warning when importing readline (if it has not yet been imported). (cherry picked from commit 599be68) Co-authored-by: Thomas Grainger <tagrain@gmail.com>

pythongh-128816: fix an import warning in test_doctest.py when import…

6083317

…ing readline in WASI

bedevere-app Bot added the tests Tests in the Lib/test dir label Jan 14, 2025

bedevere-app Bot mentioned this pull request Jan 14, 2025

test_doctests failure when running tests with -werror on platforms without readline (win32, wasi) #128816

Closed

graingert added the skip news label Jan 14, 2025

graingert requested a review from serhiy-storchaka January 14, 2025 09:05

graingert marked this pull request as ready for review January 14, 2025 09:05

bedevere-app Bot added the awaiting review label Jan 14, 2025

serhiy-storchaka approved these changes Jan 14, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Jan 14, 2025

graingert commented Jan 14, 2025

View reviewed changes

Comment thread Lib/test/test_doctest/test_doctest.py

graingert commented Jan 14, 2025

View reviewed changes

Comment thread Lib/test/test_doctest/test_doctest.py

graingert added 2 commits January 14, 2025 12:01

Apply suggestions from code review

7691ef3

Merge branch 'main' into fix-test-doctest-warning

f0e6634

graingert commented Jan 14, 2025

View reviewed changes

Comment thread Lib/test/test_doctest/test_doctest.py Outdated

Apply suggestions from code review

f80983c

graingert commented Jan 14, 2025

View reviewed changes

Comment thread Lib/test/test_doctest/test_doctest.py Outdated

Update Lib/test/test_doctest/test_doctest.py

9604504

Merge branch 'main' into fix-test-doctest-warning

e016ea4

graingert added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Jan 14, 2025

Merge branch 'main' into fix-test-doctest-warning

eeb9e8e

graingert requested a review from serhiy-storchaka January 15, 2025 05:17

serhiy-storchaka approved these changes Jan 15, 2025

View reviewed changes

serhiy-storchaka requested changes Jan 15, 2025

View reviewed changes

bedevere-app Bot added awaiting changes and removed awaiting merge labels Jan 15, 2025

graingert added 2 commits January 15, 2025 10:32

fix loader ABC warning by avoiding the abc

748a21c

use importlib.resources to acquire doctest resources

9b850e7

Revert "use importlib.resources to acquire doctest resources"

abfd936

This reverts commit 9b850e7.

bedevere-app Bot added awaiting change review and removed awaiting changes labels Jan 15, 2025

bedevere-app Bot requested a review from serhiy-storchaka January 15, 2025 10:46

serhiy-storchaka approved these changes Jan 15, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting change review labels Jan 15, 2025

serhiy-storchaka merged commit 599be68 into python:main Jan 15, 2025

bedevere-app Bot removed the awaiting merge label Jan 15, 2025

graingert deleted the fix-test-doctest-warning branch January 15, 2025 13:18

ambv added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes and removed needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Jan 15, 2025

bedevere-app Bot removed the needs backport to 3.13 only security fixes label Jan 15, 2025

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 15, 2025

Conversation

graingert commented Jan 14, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

graingert commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

graingert commented Jan 14, 2025

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

graingert commented Jan 15, 2025

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

graingert commented Jan 15, 2025

Uh oh!

graingert commented Jan 15, 2025

Uh oh!

bedevere-app Bot commented Jan 15, 2025

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app Bot commented Jan 15, 2025

Uh oh!

miss-islington-app Bot commented Jan 15, 2025

Uh oh!

miss-islington-app Bot commented Jan 15, 2025

Uh oh!

bedevere-app Bot commented Jan 15, 2025

Uh oh!

bedevere-app Bot commented Jan 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

graingert commented Jan 14, 2025 •

edited by bedevere-app Bot

Loading

graingert commented Jan 14, 2025 •

edited

Loading