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-121351: skip test_not_wiping_history_file() if no readline #121422

Merged
merged 2 commits into from
Jul 6, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Jul 6, 2024

@skirpichev

This comment was marked as outdated.

@skirpichev skirpichev requested a review from vstinner July 6, 2024 11:31
@vstinner
Copy link
Member

vstinner commented Jul 6, 2024

!buildbot AMD64 FreeBSD14

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit f41875a 🤖

The command will test the builders whose names match following regular expression: AMD64 FreeBSD14

The builders matched are:

  • AMD64 FreeBSD14 PR

@vstinner
Copy link
Member

vstinner commented Jul 6, 2024

!buildbot ARM64 MacOS M1 NoGIL

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit f41875a 🤖

The command will test the builders whose names match following regular expression: ARM64 MacOS M1 NoGIL

The builders matched are:

  • ARM64 MacOS M1 NoGIL PR

@vstinner
Copy link
Member

vstinner commented Jul 6, 2024

!buildbot s390x Fedora PR

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit f41875a 🤖

The command will test the builders whose names match following regular expression: s390x Fedora PR

The builders matched are:

  • s390x Fedora PR

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. But let's see if it does fix the issue on buildbots.

@vstinner vstinner merged commit 68e279b into python:main Jul 6, 2024
141 of 160 checks passed
@vstinner vstinner added the needs backport to 3.13 bugs and security fixes label Jul 6, 2024
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 6, 2024
…ythonGH-121422)

(cherry picked from commit 68e279b)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jul 6, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 6, 2024
vstinner pushed a commit that referenced this pull request Jul 6, 2024
…H-121422) (#121449)

gh-121351: Skip test_not_wiping_history_file() if no readline (GH-121422)
(cherry picked from commit 68e279b)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@skirpichev skirpichev deleted the test_pyrepl-121351 branch July 7, 2024 01:35
@smontanaro
Copy link
Contributor

I still see this failure on my Mac. I changed the failing test to skip it if readline.backend == "editline". I'm not sure that's the right change, but it would seem that editline and the real readline must differ in some way related to this.

diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py
index 015b6905662..0f055edfcc8 100644
--- a/Lib/test/test_pyrepl/test_pyrepl.py
+++ b/Lib/test/test_pyrepl/test_pyrepl.py
@@ -904,8 +904,10 @@ def test_python_basic_repl(self):
 
     def test_not_wiping_history_file(self):
         # skip, if readline module is not available
-        import_module('readline')
+        readline = import_module('readline')
 
+        if readline.backend == "editline":
+            self.skipTest("editline's history mgmt isn't 100% compatible w/readline")
         hfile = tempfile.NamedTemporaryFile(delete=False)
         self.addCleanup(unlink, hfile.name)
         env = os.environ.copy()

If you think that is the right thing to do, I will create a PR, but I'm kind of busy with some other stuff right now. Might take a couple days to get around to it.

@skirpichev
Copy link
Member Author

I still see this failure on my Mac.

Here is the recent test on main: https://buildbot.python.org/#/builders/1270/builds/2021 So, I doubt it's a good idea just disable this on Mac.

@smontanaro, could you, please, test original issue #121245? I worry it might be valid on your system.

Is it possible that you have some PYTHON* variables defined?

@smontanaro
Copy link
Contributor

Rebuilding the readline module to use the readline 8.2.10 library in /opt/homebrew, I still get the same failure.

Note, however, that if I unset PYTHONSTARTUP, with the current code, the failing test passes. I believe my personal readline setup is interfering. @vstinner modified another readline test to run in isolated mode (see #121414). It would seem the same change needs to be applied here.

@vstinner
Copy link
Member

Note, however, that if I unset PYTHONSTARTUP, with the current code, the failing test passes.

See #121672 in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants