Skip to content
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.

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

New test_ttk failure on Mac: "bad screen distance" #124378

Closed
smontanaro opened this issue Sep 23, 2024 · 16 comments
Closed

New test_ttk failure on Mac: "bad screen distance" #124378

smontanaro opened this issue Sep 23, 2024 · 16 comments
Labels
topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@smontanaro
Copy link
Contributor

smontanaro commented Sep 23, 2024

Bug report

Bug description:

I've been running with some patches from @ronaldoussoren to the following:

	modified:   Lib/test/support/__init__.py
	modified:   Lib/test/test_ttk/test_style.py
	modified:   Lib/test/test_ttk/test_widgets.py

(See attached diff.)
test_ttk.txt

Everything was working fine, but today I began to get a repeatable failure in test_ttk:

======================================================================
FAIL: test_configure_height (test.test_ttk.test_widgets.NotebookTest.test_configure_height)
----------------------------------------------------------------------
_tkinter.TclError: bad screen distance ""

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/skip/src/python/cpython/Lib/test/test_tkinter/widget_tests.py", line 539, in test_configure_height
    self.checkIntegerParam(widget, 'height', 100, -100, 0)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/skip/src/python/cpython/Lib/test/test_tkinter/widget_tests.py", line 81, in checkIntegerParam
    self.checkInvalidParam(widget, name, '', errmsg=errmsg)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/skip/src/python/cpython/Lib/test/test_tkinter/widget_tests.py", line 67, in checkInvalidParam
    with self.assertRaisesRegex(tkinter.TclError, errmsg or ''):
         ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: "\Aexpected integer but got ""\Z" does not match "bad screen distance """

======================================================================
FAIL: test_configure_width (test.test_ttk.test_widgets.NotebookTest.test_configure_width)
----------------------------------------------------------------------
_tkinter.TclError: bad screen distance ""

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/skip/src/python/cpython/Lib/test/test_tkinter/widget_tests.py", line 543, in test_configure_width
    self.checkIntegerParam(widget, 'width', 402, -402, 0)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/skip/src/python/cpython/Lib/test/test_tkinter/widget_tests.py", line 81, in checkIntegerParam
    self.checkInvalidParam(widget, name, '', errmsg=errmsg)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/skip/src/python/cpython/Lib/test/test_tkinter/widget_tests.py", line 67, in checkInvalidParam
    with self.assertRaisesRegex(tkinter.TclError, errmsg or ''):
         ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: "\Aexpected integer but got ""\Z" does not match "bad screen distance """

----------------------------------------------------------------------
Ran 342 tests in 6.364s

I'm putting this out there with no further investigation. I doubt it will be a release blocker for 3.13, but if so, maybe the sprinters can find and fix the problem quickly.

CPython versions tested on:

3.12, 3.13, CPython main branch

Operating systems tested on:

macOS

Linked PRs

@smontanaro smontanaro added the type-bug An unexpected behavior, bug, or error label Sep 23, 2024
@smontanaro smontanaro changed the title New test_ttk failure on Mac New test_ttk failure on Mac: "bad screen distance" Sep 23, 2024
@terryjreedy
Copy link
Member

The patch only touches the test_ttk files (and a support.init gui check) while the error is in test_tkinter. On my mac Catalina, `python3.13 -n test -ugui test_tkinter' runs without error. test_ttk has 2 or 3 failures different from the above, 1 in tkinter.

@zware
Copy link
Member

zware commented Sep 24, 2024

@smontanaro Did you happen to just update to Tcl/Tk 8.6.15? I'm seeing the same failure on Windows in GH-124449. Unfortunately it does not seem to be addressed by @culler's GH-124156 (which is for gh-124111).

@terryjreedy:

while the error is in test_tkinter.

It is actually a test_ttk.test_widgets failure, in some test methods from test_tkinter.widget_tests.

@smontanaro
Copy link
Contributor Author

@zware Yes, 8.6.15 via Homebrew, updated Sep 20:

otool -L Modules/_tkinter.cpython-314t-darwin.so
Modules/_tkinter.cpython-314t-darwin.so:
	/opt/homebrew/opt/tcl-tk/lib/libtk8.6.dylib (compatibility version 8.6.0, current version 8.6.15)
	/opt/homebrew/opt/tcl-tk/lib/libtcl8.6.dylib (compatibility version 8.6.0, current version 8.6.15)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1345.100.2)

I'm doing nothing to specify the Homebrew location. I see nothing which would be Apple-specific. Is that okay? I also a have slightly older version in my Python 3.12 Conda environment. I'll see if I can link to that.

@smontanaro
Copy link
Contributor Author

Doesn't look like I can compile/link against the miniconda3 stuff:

% gcc -bundle -undefined dynamic_lookup      Modules/_tkinter.o Modules/tkappinit.o -L/Users/skip/miniconda3/envs/python312/lib -ltk8.6 -ltkstub8.6 -ltcl8.6 -ltclstub8.6  -o Modules/_tkinter.cpython-314t-darwin.so
 
ld: warning: object file (/Users/skip/src/python/cpython/Modules/_tkinter.o) was built for newer 'macOS' version (14.6) than being linked (14.0)
ld: warning: object file (/Users/skip/src/python/cpython/Modules/tkappinit.o) was built for newer 'macOS' version (14.6) than being linked (14.0)

The resulting so file fails to import:

ERROR] _tkinter failed to import: dlopen(/Users/skip/src/python/cpython/build/lib.macosx-14.6-arm64-3.14/_tkinter.cpython-314t-darwin.so, 0x0002): Library not loaded: @rpath/libtk8.6.dylib
  Referenced from: <8EFFE0FA-0FD4-3E70-8AFD-11143C118A56> /Users/skip/src/python/cpython/Modules/_tkinter.cpython-314t-darwin.so
  Reason: no LC_RPATH's found
Following modules built successfully but were removed because they could not be imported:
_tkinter                                                                   

@zware
Copy link
Member

zware commented Sep 24, 2024

That's good news, actually :). Something seems to have changed between 8.6.14 and 8.6.15 in ttk's Notebook, so we just need to adjust our tests.

@culler
Copy link
Contributor

culler commented Sep 24, 2024 via email

@zware
Copy link
Member

zware commented Sep 24, 2024

@zware, when you say that it is not addressed do you mean because GH-124156 is for 3.14? (Because I do recall fixing that error.)

I'm double checking, but I was still seeing the failure on Windows with 8.6.15 using your GH-124156 branch merged into my GH-124449 branch.

@culler
Copy link
Contributor

culler commented Sep 24, 2024 via email

@zware
Copy link
Member

zware commented Sep 24, 2024

Ah ha. How likely is an 8.6.16 with that fix? :)

@culler
Copy link
Contributor

culler commented Sep 24, 2024 via email

@culler
Copy link
Contributor

culler commented Sep 25, 2024 via email

@culler
Copy link
Contributor

culler commented Sep 25, 2024

I can explain the cause of this problem. There was a change to the Notebook widget after the release of 8.6.14 (the change was in February 2023) which made the Notebook's width and height become "pixel" options rather than integer options. A pixel option can accept values such as "10c" (i.e 10 centimeters). As far as the tkinter test suite is concerned, this means that checkPixelsParam should be used instead of checkIntegerParam. Making just that change in the test_configure_width and test_configure_height methods of the NotebookTest class makes these failures go away. In my PR I did exactly that, but only for 9.0, not for 8.6.

However, this "solution" has drawbacks. Namely, it does not test whether it actually works to set the width of a Notebook widget to "10c", because those two methods only test with integer values. Testing pixel options causes additional issues because, at the moment, the various widgets treat these options inconsistently. Setting an option to "10c" with configure and retrieving it with cget sometimes produces "10c" and sometimes produces the number of pixels, at the current screen resolution, which are needed to span a distance of 10 cm. The former behavior is correct (since screen resolutions can change any time) but not all widgets have yet been converted to have the correct behavior. That change is happening widget by widget at the moment.

@culler
Copy link
Contributor

culler commented Sep 25, 2024

I think this is now fixed in GH-124156 (even though the thread sanitizer check seems to have failed - the failure seems unrelated to my changes).

zware added a commit to zware/cpython that referenced this issue Sep 25, 2024
Co-Authored-By: Marc Culler <culler@users.noreply.github.com>
zware added a commit that referenced this issue Sep 25, 2024
Co-authored-by: Marc Culler <culler@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 25, 2024
(cherry picked from commit fb6bd31)

Co-authored-by: Zachary Ware <zach@python.org>
Co-authored-by: Marc Culler <culler@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 25, 2024
(cherry picked from commit fb6bd31)

Co-authored-by: Zachary Ware <zach@python.org>
Co-authored-by: Marc Culler <culler@users.noreply.github.com>
@zware
Copy link
Member

zware commented Sep 25, 2024

@smontanaro, can you confirm that GH-124542 fixed these test_ttk failures for you? I expect it should, but confirmation would be nice before merging the backports :)

emilyemorehouse added a commit to lysnikolaou/cpython that referenced this issue Sep 26, 2024
* main: (69 commits)
  Add "annotate" SET_FUNCTION_ATTRIBUTE bit to dis. (python#124566)
  pythongh-124412: Add helpers for converting annotations to source format (python#124551)
  pythongh-119180: Disallow instantiation of ConstEvaluator objects (python#124561)
  For-else deserves its own section in the tutorial (python#123946)
  Add 3.13 as a version option to the crash issue template (python#124560)
  pythongh-123242: Note that type.__annotations__ may not exist (python#124557)
  pythongh-119180: Make FORWARDREF format look at __annotations__ first (python#124479)
  pythonGH-58058: Add quick reference for `ArgumentParser` to argparse docs (pythongh-124227)
  pythongh-41431: Add `datetime.time.strptime()` and `datetime.date.strptime()` (python#120752)
  pythongh-102450: Add ISO-8601 alternative for midnight to `fromisoformat()` calls. (python#105856)
  pythongh-124370: Add "howto" for free-threaded Python (python#124371)
  pythongh-121277: Allow `.. versionadded:: next` in docs (pythonGH-121278)
  pythongh-119400:  make_ssl_certs: update reference test data automatically, pass in expiration dates as parameters python#119400  (pythonGH-119401)
  pythongh-119180: Avoid going through AST and eval() when possible in annotationlib (python#124337)
  pythongh-124448: Update Windows builds to use Tcl/Tk 8.6.15 (pythonGH-124449)
  pythongh-123884 Tee of tee was not producing n independent iterators (pythongh-124490)
  pythongh-124378: Update test_ttk for Tcl/Tk 8.6.15 (pythonGH-124542)
  pythongh-124513: Check args in framelocalsproxy_new() (python#124515)
  pythongh-101100: Add a table of class attributes to the "Custom classes" section of the data model docs (python#124480)
  Doc: Use ``major.minor`` for documentation distribution archive filenames (python#124489)
  ...
@smontanaro
Copy link
Contributor Author

@smontanaro, can you confirm that GH-124542 fixed these test_ttk failures for you? I expect it should, but confirmation would be nice before merging the backports :)

Confirmed, though it does appear the PR was applied before I got to it.

@zware
Copy link
Member

zware commented Sep 26, 2024

Thanks, Skip. I was confident enough to go ahead with the main merge, but wanted to be sure before merging the backports :)

zware added a commit that referenced this issue Sep 26, 2024
(cherry picked from commit fb6bd31)

Co-authored-by: Zachary Ware <zach@python.org>
Co-authored-by: Marc Culler <culler@users.noreply.github.com>
Yhg1s pushed a commit that referenced this issue Sep 30, 2024
)

gh-124378: Update test_ttk for Tcl/Tk 8.6.15 (GH-124542)
(cherry picked from commit fb6bd31)

Co-authored-by: Zachary Ware <zach@python.org>
Co-authored-by: Marc Culler <culler@users.noreply.github.com>
@zware zware closed this as completed Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-tkinter type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants