Tk Source Code

View Ticket
Login
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: