bpo-29854: test_readline logs versions in verbose mode#2619
bpo-29854: test_readline logs versions in verbose mode#2619vstinner merged 1 commit intopython:masterfrom vstinner:test_readline_version
Conversation
|
See also @berkerpeksag PR which logs readline version in the regrtest header: PR #2618. |
Lib/test/test_readline.py
Outdated
There was a problem hiding this comment.
We add these with:
PyModule_AddIntConstant(m, "_READLINE_VERSION", RL_READLINE_VERSION);
So maybe we don't need the hasattr check?
Also, we need to know this is libedit, and looking at the tests, we need:
is_editline = readline.__doc__ and "libedit" in readline.__doc__
There was a problem hiding this comment.
So maybe we don't need the hasattr check?
I like the idea of using test_readline unchanged on Python implementations other than CPython. Tests cannot rely on private attributes in tests, not without @cpython_only or hasattr(). I added a comment to explain the rationale.
is_editline
Right, I also logged this flag, even if the other attributes are not available.
I chose to also expose rl_library_version as a private attribute to be able to log it in test_readline. It may help to debug subtle differences between two readline versions, and validate manually is_editline flag.
Lib/test/test_readline.py
Outdated
There was a problem hiding this comment.
@berkerpeksag version showing "readline implementation: libedit|GNU readline" can be little nicer.
Modules/readline.c
Outdated
There was a problem hiding this comment.
Why not uppercase like the other constants?
There was a problem hiding this comment.
Oh, I didn't notice that the variable on the previous line is in lower case but exported as UPPER case. I converted my new constant to UPPER case and rename "rl" to READLINE, as done for the 2nd constant.
* test_readline logs the versions of libreadline when run in verbose mode * Add also readline._READLINE_LIBRARY_VERSION
| if hasattr(readline, "_READLINE_LIBRARY_VERSION"): | ||
| is_editline = ("EditLine wrapper" in readline._READLINE_LIBRARY_VERSION) | ||
| else: | ||
| is_editline = (readline.__doc__ and "libedit" in readline.__doc__) |
There was a problem hiding this comment.
I don't like duplicating the horrible doc check, and adding a new incompatible check for the same thing.
There was a problem hiding this comment.
I don't like duplicating the horrible doc check, and adding a new incompatible check for the same thing.
IMHO such enhancement would deserves its own PR ;-)
3 participants
No description provided.