Ticket UUID: | 48ff50099931a4f935a5b8e945653cd660ec2f04 | |||
Title: | reworked textWind.test : font agnostic | |||
Type: | Bug | Version: | revised_text | |
Submitter: | bll | Created on: | 2017-07-17 16:31:08 | |
Subsystem: | 18. [text] | Assigned To: | fvogel | |
Priority: | 5 Medium | Severity: | Minor | |
Status: | Closed | Last Modified: | 2018-10-06 16:28:57 | |
Resolution: | Fixed | Closed By: | fvogel | |
Closed on: | 2018-10-06 16:28:57 | |||
Description: |
Attached is a new textWind.test file. All of the font constraints have been removed and the tests updated to calculate the proper result. Passes on Linux, Mac OS X, Windows | |||
User Comments: |
fvogel added on 2018-10-06 16:28:57:
Merged to core-8-6-branch, trunk and revised_text. fvogel added on 2018-10-06 12:36:45: Exact same perfect results on Linux Debian 8 as on macOS reported below (including textWind-10.8 that does no longer fail). Brad: yes, this is the easy one. The other one (textDisp.c) is at [44e92d896e]. bll added on 2018-10-03 22:03:24: Gosh, it's been more than a year. font agnostic maybe referred more to different fonts across different platforms? I don't recall. Looks like 10.8 is dependent on timing -- maybe someone can come up with a better way to write it. I hope the merge with any changed tests is not too difficult. I seem to recall there's another .test file I updated for revised_text. This is the easy one? One of the two was much more difficult. fvogel added on 2018-10-03 21:53:15: Also very good results for macOS: core-8-6-branch: Total 110 Passed 67 Skipped 42 Failed 1 bugfix branch: Total 110 Passed 110 Skipped 0 Failed 0 The failing test in core-8-6-branch was: ==== textWind-10.8 EmbWinLayoutProc procedure, error in creating window FAILED ==== Contents of test case: .t insert 1.0 "Some sample text" .t window create 1.5 -create { toplevel .t2 -width 100 -height 150 wm geom .t2 +0+0 concat .t2 } set msg {} update set i 0 while {[llength $msg] == 1 && [incr i] < 200} { update } return $msg ---- Result was: {{can't embed .t2 relative to .t}} ---- Result should have been (exact matching): {{can't embed .t2 relative to .t}} {{window name "t2" already exists in parent}} ==== textWind-10.8 FAILED And a note to myself I forgot to insert in my previous message: The patch changes some tests as follows: - if {[lsearch -exact $msg $args] == -1} { - lappend msg $args - } + lappend msg $args This change is OK for all tests except textWind-10.6. If I put this in that test then it fails unreliably (approx. 2/3 of the runs). There must be a race condition somewhere in the legacy text widget code. This is most likely not the case with revised_text. fvogel added on 2018-10-03 21:35:31: Thank you for this contribution! I have placed your textWind file in a bugfix branch for easy testing. I thought that your contribution could benefit not only to revised_text but also to the legacy text widget, therefore the bugfix branch is off core-8-6-branch and I will merge forward when it's ready. A few notes (mainly for myself, all this on Windows): - There are still some hardcoded numbers in the expected results. From my analysis on just the first test, they come from the size of the frame (3x3). Perhaps this could be improved (originally I wondered why there were still hardcoded figures there). - "font agnostic"? not really. If I change the font used from {"Courier New" -12} to {"Courier New" -14} or {"Lucida Console" -12} then some tests start to fail. However I think we don't care, since we have much improved results in terms of tests coverage, for instance this is obtained for me on Windows Vista:core-8-6-branch: Total 110 Passed 74 Skipped 36 Failed 0 bugfix branch: Total 110 Passed 110 Skipped 0 Failed 0 I'll check and confirm on Linux and macOS as well. |
Attachments:
- textWind.test [download] added by bll on 2017-07-17 16:31:41. [details]