8000 test_webbrowser failure on MacOS if BROWSER set to "open" · Issue #131254 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

test_webbrowser failure on MacOS if BROWSER set to "open" #131254

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.

Sign up for GitHub

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

Jump to bottom
Closed
smontanaro opened this issue Mar 14, 2025 · 12 comments
Closed

test_webbrowser failure on MacOS if BROWSER set to "open" #131254

smontanaro opened this issue Mar 14, 2025 · 12 comments
Labels
3.14 bugs and security fixes OS-mac tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@smontanaro
Copy link
Contributor
smontanaro commented Mar 14, 2025 •
edited by picnixz
Loading

Bug report

Bug description:

I just noticed a new-to-me failure on main for the test_webbrowser unit test. Here's the rundown:

% ./python.exe -E  -m test test_webbrowser
Using random seed: 3730311481
Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 4.38 Run 1 test sequentially in a single process
0:00:00 load avg: 4.38 [1/1] test_webbrowser
test test_webbrowser failed -- Traceback (most recent call last):
  File "/Users/skip/src/python/cpython/Lib/test/test_webbrowser.py", line 334, in test_default
    assert isinstance(browser, webbrowser.MacOSXOSAScript)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

0:00:00 load avg: 4.38 [1/1/1] test_webbrowser failed (1 failure)

== Tests result: FAILURE ==

1 test failed:
    test_webbrowser

Total duration: 48 ms
Total tests: run=48 failures=1 skipped=4
Total test files: run=1/1 failed=1
Result: FAILURE
% env | grep BROWSER
BROWSER=open
% unset BROWSER     
% ./python.exe -E  -m test test_webbrowser
Using random seed: 2541583609
Raised RLIMIT_NOFILE: 256 -> 1024
0:00:00 load avg: 3.19 Run 1 test sequentially in a single process
0:00:00 load avg: 3.19 [1/1] test_webbrowser
0:00:00 load avg: 3.19 [1/1] test_webbrowser passed

== Tests result: SUCCESS ==

1 test OK.

Total duration: 50 ms
Total tests: run=48 skipped=4
Total test files: run=1/1
Result: SUCCESS

In short, if the BROWSER environment variable is set to open, the test fails. (This didn't used to be the case as far as I can recall. The entire test suite passed a couple days ago with the expected skips, no failures.) If I unset BROWSER, the test succeeds.

I suspect this commit is the culprit: 96492785b202a92af1b71f8c011ea839ca3ebb07

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

Feature was introduced in #130535.
@smontanaro smontanaro added the type-bug An unexpected behavior, bug, or error label Mar 14, 2025
@smontanaro
Copy link
Contributor Author

@minrk

@picnixz picnixz added tests Tests in the Lib/test dir OS-mac labels Mar 14, 2025
@picnixz
Copy link
Member
picnixz commented Mar 14, 2025 •
edited
Loading

AFAIU, I suggest using EnvironmentVarGuard and unset the env var, leaving it to an unset BROWSER.

@ned-deily
Copy link
Member

The test should guard as @picnixz suggests. But just out of curiosity, why is BROWSER set to open in the first place? Is it for something other than python?

@smontanaro
Copy link
Contributor Author

But just out of curiosity, why is BROWSER set to open in the first place? Is it for something other than python?

That's the most obvious choice to me on MacOS. I don't need to include a full path to a browser (Brave, Safari, whatever). To wit:

% type safari
safari not found
% type Safari
Safari not found

If I want to switch to a different default browser, MacOS allows me to do that in the settings app. No need to mess with any environment variables.

@bedevere-app bedevere-app bot mentioned this issue Mar 15, 2025
Merged
@minrk
Copy link
Contributor
minrk commented Mar 15, 2025

Thanks for fixing the test, @picnixz! Sorry for missing it the first time around.

@smontanaro I think the question was about why override BROWSER to open explicitly when this already matches the default behavior, unless the reason is for something other than Python where maybe it doesn't match.

@smontanaro
Copy link
Contributor Author
smontanaro commented Mar 15, 2025 •
edited
Loading

@smontanaro I think the question was about why override BROWSER to open explicitly when this already matches the default behavior, unless the reason is for something other than Python where maybe it doesn't match.

I'm not entirely sure, but... I have the same setting on my old Dell (predecessor to my Mac). It runs Ubuntu, which has /usr/bin/open. It's likely the setting just came along for the ride. (I keep my config files in Git.) I don't think it's Python-specific either:

https://askubuntu.com/questions/27447/google-earth-and-browser-environment-variable

Edit: Which is to say maybe there are other tools which use BROWSER but don't default to open if unset.

@picnixz
Copy link
Member
picnixz commented Mar 16, 2025

@smontanaro Can you have a look at the PR to check if it works? I can't request you for review. TiA

@smontanaro
Copy link
Contributor Author

I looked at the diff, but the code in that section of my latest pull didn't look at all like it. I assumed it had already been merged to main, so didn't try. In any case, test_webbrowser passes once again on main, so it seems the fix worked.

@picnixz
Copy link
Member
picnixz commented Mar 16, 2025

I assumed it had already been merged to main, so didn't try.

I don't think I've merged it but maybe it's because of some side-effects that it's still passing? anyway, I think it doesn't hurt to simply unset the environment variable temporarily. Or @hugovk (you're the only other one I know who's using a mac, sorry for the ping...) maybe you could check if it passes on your mac if you do BROWSER=open python -m test test_webbrowser?

@smontanaro
Copy link
Contributor Author

I looked at the diff, but the code in that section of my latest pull didn't look at all like it. I assumed it had already been merged to main, so didn't try. In any case, test_webbrowser passes once again on main, so it seems the fix worked.

Ack! I had unset BROWSER at one point, so that's why it was passing...

I can confirm that applying the change from your PR fixes the test.

@hugovk
Copy link
Member
hugovk commented Mar 16, 2025

Or @hugovk (you're the only other one I know who's using a mac, sorry for the ping...) maybe you could check if it passes on your mac if you do BROWSER=open python -m test test_webbrowser?

Likewise confirmed!

picnixz added a commit that referenced this issue Mar 16, 2025
@picnixz
gh-131254: ensure that BROWSER is not set for test_webbrowser on …
9b6cef0 
…macOS (#131276)
@picnixz picnixz added the 3.14 bugs and security fixes label Mar 16, 2025
@picnixz
Copy link
Member
picnixz commented Mar 16, 2025

As I said on the PR, no need for bp as the test was added in #130535. Thanks for the report and for testing my fix!

@picnixz picnixz closed this as completed Mar 16, 2025
plashchynski pushed a commit to plashchynski/cpython that referenced this issue Mar 17, 2025
@picnixz @plashchynski
pythongh-131254: ensure that BROWSER is not set for `test_webbrowse…
b4bf7df 
…r` on macOS (python#131276)
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@picnixz @seehwan80
pythongh-131254: ensure that BROWSER is not set for `test_webbrowse…
b8c1202 
…r` on macOS (python#131276)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.14 bugs and security fixes OS-mac tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@minrk @hugovk @smontanaro @ned-deily @picnixz
0