Skip to content
This repository has been archived by the owner on Dec 28, 2023. It is now read-only.

Tests rely on Helvetica #10

Closed
PurpleMyst opened this issue May 25, 2019 · 5 comments
Closed

Tests rely on Helvetica #10

PurpleMyst opened this issue May 25, 2019 · 5 comments
Labels
bug good first issue A relatively easy issue to fix for those unfamiliar with the code base

Comments

@PurpleMyst
Copy link
Collaborator

Most of the tests in tests/test_font.py rely on the font Helvetica being present.

This is not the case for all *nix installations, although admittedly it is the case for most Linux personal computers.

A fix for this is to utilize a program such as xlsfonts to figure out which fonts are installed and choose one with a correct min/max size bound.

@PurpleMyst PurpleMyst added bug good first issue A relatively easy issue to fix for those unfamiliar with the code base labels May 25, 2019
@Akuli
Copy link
Owner

Akuli commented May 29, 2019

xlsfonts output lines don't seem to be the same thing as Tk's font families. On this system, one of the output lines is 10x20, but I get this:

>>> f = teek.Font(('10x20', 12, ''))
>>> f.family     # seems to be a fallback
'DejaVu Sans'
>>> f = teek.Font(('asdasjfioajsf', 12, ''))
>>> f.family
'DejaVu Sans'
>>> 

A better solution might be to pick a font family from the teek.Font.families list:

>>> teek.Font.families()
['Latin Modern Mono Slanted', 'MathJax_Caligraphic', ..., 'DejaVu Sans', ...]

Something like random.choice(teek.Font.families()) wouldn't be good, because then the tests might fail sometimes and succeed at other times, but something like min(teek.Font.families()) for the alphabetically first font might work.

@Akuli
Copy link
Owner

Akuli commented May 29, 2019

@PurpleMyst, does teek.Font.families() contain Helvetica on the system where the Helvetica tests didn't work? For me, the asserts in this code don't fail:

>>> for family in teek.Font.families():
...     font = teek.Font((family, 42, ''))
...     assert font.family == family
...     assert font.size == 42
... 
>>> 

@codetent
Copy link
Contributor

codetent commented Jun 1, 2019

Is it a problem if Helvetica is not installed? If the given font family does not exist on the system, the most closely matching native font is selected by tk.

Besides, it would be a better solution to use TkDefaultFont instead of Helvetica because this is the font family which is used by tkinter for the tests.

@Akuli if the font is not installed on the system, it is not in the list obtained by teek.Font.families().

@Akuli
Copy link
Owner

Akuli commented Jun 2, 2019

The problem is that when Tk selects the most closely matching font, it may differ from the font passed to teek.Font so much that the font size comes out as different. (*) Myst had a situation where teek.Font(('Helvetica', 12)).size was not 12. However, teek.Font(('utopia', 12)).size was 12 on that system. I guess anything else from teek.Font.families() would have worked too, and if that's true, then the tests should be changed to use something like that.

TkDefaultFont is not actually a font family; it is a name of a named font. Code like teek.Font(('TkDefaultFont', 12)) is "wrong", but it works nicely anyway because Tk looks for a font family called TkDefaultFont, and doesn't find it, then falls back to whatever the default font family is. The correct TkDefaultFont code would be teek.Font('TkDefaultFont') or teek.NamedFont('TkDefaultFont'). Notice that no size can be specified here, because again, TkDefaultFont is not a font family.

(*) Edit: This is because teek's .size property uses font actual documented in font(3tk) to look up the family and size. I thought this would be a good idea when I made this. I can add a separate .actual() method or something if it's not a good idea after all.

@Akuli
Copy link
Owner

Akuli commented Jun 20, 2019

this is now fixed in #11

@Akuli Akuli closed this as completed Jun 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug good first issue A relatively easy issue to fix for those unfamiliar with the code base
Projects
None yet
Development

No branches or pull requests

3 participants