classification
Title: webbrowser._synthesize uses outdated calling signature for webbrowser.register
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: daves, jmsdvl, miss-islington, ncoghlan, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2017-07-24 14:32 by jmsdvl, last changed 2018-11-26 21:49 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 2689 closed jmsdvl, 2017-07-24 14:32
PR 7267 merged serhiy.storchaka, 2018-05-31 04:28
PR 8183 merged miss-islington, 2018-07-08 07:23
PR 10729 merged miss-islington, 2018-11-26 21:29
Messages (13)
msg298976 - (view) Author: John Still (jmsdvl) * Date: 2017-07-24 14:32
The function `register` of the `webbrowser` module was updated a little while back to use the newish keyword-only argument syntax; however, the function `_synthesize` (also in `webbrowser`) is still using the outdated positional arguments only calling convention; leading a pair of tests in `Lib/tests/test_webbrowser.py` to fail.  I've issued a PR that fixes the function call to use the more modern calling convention.
msg298986 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-07-24 16:06
Is it possible to write a test?
msg298987 - (view) Author: John Still (jmsdvl) * Date: 2017-07-24 16:19
What would a new test be testing? I only found this behaviour because two existing tests were failing (with the PR they pass, of course). I'm happy to write a test, I'm just not sure what the test should be testing that the existing tests don't already test.
msg298989 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-07-24 16:30
These tests are not failing on our buildbots and developer's computers. Seems they are failing only in special environment. This path of execution is not tested in more common environments, therefore possible regression couldn't be caught until somebody run tests in similar special environment. New test should test this path of execution in common environments.
msg299000 - (view) Author: John Still (jmsdvl) * Date: 2017-07-24 17:29
Ok I understand what you mean.

I think the only conditions for test failure are (A) $BROWSER is set and (B) $BROWSER is set to an actual executable that should be caught by ``register_standard_browsers`` prior to execution reaching the last if clause of ``register_standard_browsers`` (``if 'BROWSER' in os.environ:``) - those two conditions should be sufficient for execution to reach the offending line of code.

I'll see if I can cook up a test for that.  I think the right way to do this is to monkeypatch ``os.environ`` and ``shutil.which``, am I right?
msg299007 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-07-24 18:03
This would be not easy. Since the BROWSER environment variable should be patched before the first use the webbrowser module, the test should import webbrowser in a separate process (you can use the test.support.script_helper.assert_python_ok() helper).

_synthesize() is used in two places with different arguments: in register_standard_browsers() and in get().
msg318173 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-05-30 12:12
bpo-33693 has been marked as a duplicate of this issue. It seems to be a regression caused by bpo-24241.
msg318174 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-05-30 12:15
I'm surprised that test_webbrowser pass on my Fedora 28, but it fails for Greg Walters:
https://bugs.python.org/issue33693#msg318163

Some tests are skipped depending on the OS? Maybe we should mock more things, or something else, to get a better code coverage.

I ran PR 2689 new test without the fix on webbrowser.py: the test doesn't fail. So this PR is not enough to detect bpo-33693.
msg318243 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-31 04:30
PR 7267 is based on PR 2689, but adds two tests that cover both cases of using webbrowser._synthesize().
msg321266 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-08 07:22
New changeset 25b804a9c21c735ce322877f105ebab2539ccfc1 by Serhiy Storchaka in branch 'master':
bpo-31014: Fix the webbrowser module. (GH-7267)
https://github.com/python/cpython/commit/25b804a9c21c735ce322877f105ebab2539ccfc1
msg321268 - (view) Author: miss-islington (miss-islington) Date: 2018-07-08 08:09
New changeset a410f9f614b62cd7df220186d081ffd73786be91 by Miss Islington (bot) in branch '3.7':
bpo-31014: Fix the webbrowser module. (GH-7267)
https://github.com/python/cpython/commit/a410f9f614b62cd7df220186d081ffd73786be91
msg330466 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-11-26 21:29
New changeset 8c281ed403fd915284d5bba2405d7c47f8195066 by Serhiy Storchaka (Zhiming Wang) in branch 'master':
bpo-35308: Fix regression where BROWSER env var is not respected. (GH-10693)
https://github.com/python/cpython/commit/8c281ed403fd915284d5bba2405d7c47f8195066
msg330469 - (view) Author: miss-islington (miss-islington) Date: 2018-11-26 21:49
New changeset 2a37f013ec81099a6156160ce66803b2609bb7f4 by Miss Islington (bot) in branch '3.7':
bpo-35308: Fix regression where BROWSER env var is not respected. (GH-10693)
https://github.com/python/cpython/commit/2a37f013ec81099a6156160ce66803b2609bb7f4
History
Date User Action Args
2018-11-26 21:49:33miss-islingtonsetmessages: + msg330469
2018-11-26 21:29:57miss-islingtonsetpull_requests: + pull_request9978
2018-11-26 21:29:49serhiy.storchakasetmessages: + msg330466
2018-07-08 08:19:09serhiy.storchakasetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: - Python 2.7, Python 3.6
2018-07-08 08:09:30miss-islingtonsetnosy: + miss-islington
messages: + msg321268
2018-07-08 07:23:42miss-islingtonsetpull_requests: + pull_request7738
2018-07-08 07:22:35serhiy.storchakasetmessages: + msg321266
2018-05-31 04:30:13serhiy.storchakasetassignee: serhiy.storchaka
2018-05-31 04:30:06serhiy.storchakasetmessages: + msg318243
versions: + Python 2.7, Python 3.6, Python 3.8
2018-05-31 04:28:15serhiy.storchakasetkeywords: + patch
pull_requests: + pull_request6895
2018-05-30 12:41:11ncoghlansetstage: patch review
2018-05-30 12:15:57vstinnersetmessages: + msg318174
2018-05-30 12:12:17vstinnersetnosy: + vstinner
messages: + msg318173
2018-05-30 11:28:43martin.panterlinkissue33693 superseder
2017-07-24 18:03:43serhiy.storchakasetmessages: + msg299007
2017-07-24 17:29:42jmsdvlsetmessages: + msg299000
2017-07-24 16:30:27serhiy.storchakasetmessages: + msg298989
2017-07-24 16:19:37jmsdvlsetmessages: + msg298987
2017-07-24 16:06:29serhiy.storchakasetnosy: + serhiy.storchaka, daves, ncoghlan
messages: + msg298986
2017-07-24 14:32:24jmsdvlcreate