OS X test for Tk availability doesn't work

[alex]

BPO	17496
Nosy	@ronaldoussoren, @ned-deily, @glyph, @alex, @roseman
Files	tk-cond.diff issue-17496.txt

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ronaldoussoren'
closed_at = None
created_at = <Date 2013-03-20.19:50:05.038>
labels = ['OS-mac', 'type-bug', 'expert-tkinter', '3.8', '3.7', 'tests']
title = "OS X test for Tk availability in runtktests.py doesn't work"
updated_at = <Date 2018-06-27.07:17:07.088>
user = 'https://github.com/alex'

bugs.python.org fields:

activity = <Date 2018-06-27.07:17:07.088>
actor = 'ronaldoussoren'
assignee = 'ronaldoussoren'
closed = False
closed_date = None
closer = None
components = ['macOS', 'Tests', 'Tkinter']
creation = <Date 2013-03-20.19:50:05.038>
creator = 'alex'
dependencies = []
files = ['29510', '29990']
hgrepos = []
issue_num = 17496
keywords = ['patch']
message_count = 14.0
messages = ['184784', '184843', '184844', '184845', '184846', '184850', '184851', '184862', '187632', '193592', '219025', '219028', '320500', '320551']
nosy_count = 6.0
nosy_names = ['ronaldoussoren', 'ned.deily', 'glyph', 'alex', 'markroseman', 'jesstess']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17496'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8']

Linked PRs

• gh-61698: Use launchctl to detect macOS window manager in tests #118390
• [3.13] gh-61698: Use launchctl to detect macOS window manager in tests (GH-118390) #125392
• [3.12] gh-61698: Use launchctl to detect macOS window manager in tests (GH-118390) #125393

[alex]

If I run:

$ python -mtest.test_tk

I get a skip, after speaking with people familiar with OS X, it appears that the condition for the skip uses old Carbon APIs, which are totally deprecated under 64-bit. Attached is a patch which should work.

[ned-deily]

The test for the condition was added to solve the problem reported in bpo-8716. The Tk crash for test_ttk_guionly reported there still occurs on a current 10.8 system with the Apple-supplied Cocoa Tk under the same conditions, that is, when running the tests from a process without a window manager connection, like an ssh or buildbot process where the user is not also currently logged in as the main "GUI user". And the skip test code in question does prevent that crash. The skip test also works correctly when using a 64-bit framework Python build (./configure --enable-framework), i.e. it does not skip the tests in a normal terminal session. A side effect of a framework build is that the Python interpreter runs within an OS X app bundle, which gives it magic GUI powers.

But if run from a non-framework (standard unix) build, the tests are skipped with a "cannot run without OS X gui process" skip although stubbing out the check, as your patch does, shows that test_tk and test_ttk_guionly appear to run without error. So it seems that the skip test is too restrictive but it shouldn't be unilaterally deleted.

[glyph]

Hi Ned,

It seems from your comment that you didn't read the patch. Alex added a simpler check via launchctl, rather than by framework symbol groveling :). He didn't remove the check.

It should be functionally identical to what's there now, but much shorter and without having to depend on ctypes.

-glyph

[ned-deily]

Um, yes, my tired eyes did skip over those added lines. Thanks, Glyph, and sorry, Alex.

While the suggested change solves the issue for the non-framework build case, it appears to introduce new problems. For one, with the current skip test, it is possible to run the tests from a buildbot or ssh process as long as the user name is logged in. That no longer works with the proposed change. Also, "launchctl managername" does not appear to be available on releases prior to 10.6, releases we still support.

[ned-deily]

Granted, the current test is a kludge. We could make it a bigger kludge by trying launchctl first and if it fails move on to the current ctypes-based tests. Any better options?

[ronaldoussoren]

A small helper program that does the equavalent of this should also work:

import Cocoa
Cocoa.NSWindow.alloc().initWithContentRect_styleMask_backing_defer_(((10, 10), (100, 100)), 0, 0, 0)

If this code raises an exception you cannot create windows, if it doesn't you can. This doesn't actually show the window and shouldn't mess up the users desktop when running the testsuite interactively.

[ronaldoussoren]

Wouldn't it be better to check for the actual problem, that is use subprocess to start a small Tcl script that creates a window and check if that crashes?

That way the code for disabling the test doesn't have to try to guess whether or not Tk will crash in the current environment, and tests won't get skipped when the Tk folks get their act together and don't crash when the window manager isn't available.

[glyph]

Wouldn't it be better to check for the actual problem, that is use subprocess to start a small Tcl script that creates a window and check if that crashes?

Yes, this sounds great. Doing it with Tcl means that we're not invoking any of the problematic code in question. It sounds like this check could be done on other platforms as well, in lieu of looking for e.g. $DISPLAY. If a tcl script can make tk windows, so should a Python script be able to.

[ronaldoussoren]

The attached patch start wish and stops it by sending the 'exit' command. With the patch I can run the tk tests without getting a skip. I cannot easily test if the patch also does the right thing on buildbots, but have high hopes. I did check that just starting 'wish' on a machine where I don't have console access causes a crash (OSX 10.5, nobody on the console, I logged on through SSH).

Open issues with my patch:

• The check uses whatever version wish is on the PATH, which may or may
  not be related to the version of Tk that python is linked to.

  It might be better to explicitly use /usr/bin/wish, as that one is
  known to be native port of Tk (not X11) and is known to crash.

• Not tested if the patch causes a Skip when running tests without
  GUI access.

[ronaldoussoren]

To respond to my own message: running a small Tcl script is not good enough, the process environment of python itself is also important.

In particular, GUI frameworks only work from inside an application bundle (or by using some undocumented private APIs) and that's why the Tk tests cannot work on OSX without installing.

BTW. I just verified that MacOS.WMAvailable() works just fine in 64-bit processes (python 2.7 on OSX 10.8), and according to the header files the API functions used in its implementation (and the corresponding python code in 3.x) are not deprecated.

[ned-deily]

(See also bpo-18604 which has refactored this area.)

[jesstess]

Some IRC discussion about what contributors should do while this is unresolved, and the bigger plan for comprehensively addressing this:

01:53 < ned_deily> jesstess, saw your nosy on bpo-17496. Beware that it's really a can of worms and definitely was misclassified as easy. Lots of edge cases, some not discussed in the issue.
03:01 < jesstess> ned_deily: can you update the ticket with edge cases not yet discussed? The issue is making life hard for some of our interns, so I'd love to see some progress on it.
03:02 < jesstess> ned_deily: I also found ronaldoussoren's conclusions hard to follow. I think a re-statement of what the best solution for the problem is would be really helpful, if you have opinions on it.
03:04 < ned_deily> jesstess, can you say how it is causing problems?
03:08 < jesstess> ned_deily: They are working on Tkinter tickets, and their primary development platform happens to be OSX, where the tests are getting skipped. One has set up a Linux VM already, which is fine, but it's a confusing issue to hit for new contributors and they also want to have the confidence that changes and new tests pass on OSX before submitting patches for review.
03:09 < ned_deily> jesstess, OK. Alas, Tk is a bit of a problem on any platform and it's pretty much a mess on OS X.
03:15 < ned_deily> jesstess, Currently, there are three different variants of Tk in use on OS X. And the most commonly used variant, the Cocoa Tk 8.5, is still relatively new and has had major bugs (still present in the version shipped by Apple in OS X) and has variations from minor release to minor release.
03:20 < ned_deily> jesstess, The plan is to make things a lot easier to test by making it easier to use a custom-built Tk for OS X. In the meantime, there really is no easy way around things. Fixing bpo-17496 by itself won't allow testing on OS X.
03:20 < ned_deily> jesstess, For the time being, I would suggest just testing as best as possible on whatever platforms one can and submitting the patches.

[roseman]

Just to note, this remains a PITA. To run gui tests on macOS from a terminal window seems to require commenting out the SetFrontProcess() call. A better replacement is needed as noted in the previous discussion, as well this call was deprecated in OS X 10.9 (though has not yet been removed as of this writing).

In the interim, is there a precedent for adding another command line option to the 'test' module, e.g. '--forcemacgui' so that people who want to can run those tests from a clean checkout?

[ronaldoussoren]

Do the tests work properly from an installed python framework?

That doesn't immediately help with the normal way of running tests, but might point to a way forward. The major difference with normal installs is that the python interpreter is started in a minimal .app bundle, which leads to better behaviour of Apple's GUI frameworks.

[serhiy-storchaka]

runtktests.py was removed in f59ed3c (bpo-45229).

[erlend-aasland]

runtktests.py was removed in f59ed3c (bpo-45229).

The code is now in Lib/test/support/__init__.py, and it is still a nuisance on macOS.

cpython/Lib/test/support/__init__.py

Lines 247 to 269 in 23d0371

	elif sys.platform == 'darwin':
	# The Aqua Tk implementations on OS X can abort the process if
	# being called in an environment where a window server connection
	# cannot be made, for instance when invoked by a buildbot or ssh
	# process not running under the same user id as the current console
	# user. To avoid that, raise an exception if the window manager
	# connection is not available.
	from ctypes import cdll, c_int, pointer, Structure
	from ctypes.util import find_library
	app_services = cdll.LoadLibrary(find_library("ApplicationServices"))
	if app_services.CGMainDisplayID() == 0:
	reason = "gui tests cannot run without OS X window manager"
	else:
	class ProcessSerialNumber(Structure):
	_fields_ = [("highLongOfPSN", c_int),
	("lowLongOfPSN", c_int)]
	psn = ProcessSerialNumber()
	psn_p = pointer(psn)
	if ( (app_services.GetCurrentProcess(psn_p) < 0) or
	(app_services.SetFrontProcess(psn_p) < 0) ):
	reason = "cannot run without OS X gui process"

[ronaldoussoren]

runtktests.py was removed in f59ed3c (bpo-45229).

The code is now in Lib/test/support/__init__.py, and it is still a nuisance on macOS.

cpython/Lib/test/support/__init__.py

Lines 247 to 269 in 23d0371

	elif sys.platform == 'darwin':
	# The Aqua Tk implementations on OS X can abort the process if
	# being called in an environment where a window server connection
	# cannot be made, for instance when invoked by a buildbot or ssh
	# process not running under the same user id as the current console
	# user. To avoid that, raise an exception if the window manager
	# connection is not available.
	from ctypes import cdll, c_int, pointer, Structure
	from ctypes.util import find_library
	app_services = cdll.LoadLibrary(find_library("ApplicationServices"))
	if app_services.CGMainDisplayID() == 0:
	reason = "gui tests cannot run without OS X window manager"
	else:
	class ProcessSerialNumber(Structure):
	_fields_ = [("highLongOfPSN", c_int),
	("lowLongOfPSN", c_int)]
	psn = ProcessSerialNumber()
	psn_p = pointer(psn)
	if ( (app_services.GetCurrentProcess(psn_p) < 0) or
	(app_services.SetFrontProcess(psn_p) < 0) ):
	reason = "cannot run without OS X gui process"

It might be worthwhile to check if using the output of launchctl managername is more useful. This is Aqua for GUI sessions and Background for SSH sessions. The disadvantage is that this subcommand is documented as part of the legacy subcommands, but that's not worse that using GetCurrentProcess which is explicitly deprecated.

[erlend-aasland]

launchctl managername indeed looks more useful. As you say, the man page says it is a legacy command. Quoting launchctl --help:

When using a legacy subcommand which manipulates a domain, the target domain is
inferred from the current execution context. When run as root (whether it is
via a root shell or sudo(1)), the target domain is assumed to be the
system-wide domain. When run from a normal user's shell, the target is assumed
to be the per-user domain for that current user.

We're not manipulating anything, so it seems to me we can safely use the legacy command to query the manager name.

[erlend-aasland]

Quoting Ned:

Also, "launchctl managername" does not appear to be available on releases prior to 10.6, releases we still support.

IIRC, we don't officially support 10.6 anymore.

[ronaldoussoren]

Quoting Ned:

Also, "launchctl managername" does not appear to be available on releases prior to 10.6, releases we still support.

IIRC, we don't officially support 10.6 anymore.

Not in CI at least. Anyone supporting old OS versions can keep using the old check, MacPorts already carries a number of patches to support ancient versions of macOS.

[erlend-aasland]

Something like #118390 should work; if launchctl fails, we skip with a message similar to what we have now; if it succeeds, use the result (and if the result is garbled, something is incredibly wrong, so don't fail graciously)

[erlend-aasland]

I believe we can now close this. Thanks to everyone involved.