test_webbrowser failure on MacOS if BROWSER set to "open"

[smontanaro]

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

• gh-131254: ensure that BROWSER is not set for test_webbrowser on macOS #131276

Feature was introduced in #130535.

[smontanaro]

@minrk

[picnixz]

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

[ned-deily]

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]

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.

[minrk]

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]

@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]

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

[smontanaro]

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]

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]

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]

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!