Notes |
(0011246 - 59 - 83 - 83 - 83 - 83 - 83)
wiz
10-07-07 07:23
|
Reminder sent to: RalphJohnson Hi Ralph,
I think this is one you should know about.
|
|
(0011249 - 1611 - 1703 - 1703 - 1703 - 1703 - 1703)
RalphJohnson
10-07-07 09:24
|
You are right that it depends on the state of the image, but the real problem is that it assumes that "normal emphasized: 3" and "normal derivativeFonts at: 3" mean the same thing. They don't. "normal emphasized: 3" will make sure that there is a derivative font with the right emphasis. See StrikeFont>>emphasized: for an explanation of the emphasis numbers. There is an array of all the derivative fonts, indexed by the emphasis number. However, the #derivativeFonts methods does NOT return that array! Instead, it returns a copy without the nils. Since most elements of the array are nil, the font with emphasis N is not likely to be the N'th element of the return collection of #derivativeFonts.
An easy fix is to first send "normal emphasized: 1" and "normal emphasized: 2" to make sure that the first three elements of the derivativeFonts array are not nil.
I looked for a way to find the fonts without sending #derivativeFonts, and there aren't any. It then occurred to me that the first assertion is not only wrong, but pointless. The purpose of this test is to make sure #reset works. So, and easier fix is to delete the first assertion.
In general, this is a bad test. Which class is it testing? It depends on the default font. There should be a test for each of the subclasses of AbstractFont. Right now it basically is picking one at random, based on whatever happens to be the font of the default TextStyle. In the original image, this is StrikeFontSet. So, neither StrikeFont (which has a complex #reset method) nor TTCFont (which has a trivial #reset method) are tested. |
|
(0011251 - 354 - 354 - 354 - 354 - 354 - 354)
RalphJohnson
10-07-07 14:59
|
I think that we need a FontTest class that has a setup method that creates StrikeFonts (or some other concrete class) instead of getting the default font. Then we could subclass that, change the setup method to create fonts of different classes, and reuse the font tests. This would make the tests more repeatable as well as check all the font classes. |
|
(0011252 - 1290 - 1410 - 1410 - 1410 - 1410 - 1410)
wiz
10-07-07 19:02
|
Hi Ralph,
I think that there needs to be a general #resetFontsToTestableState method. It probably right now belongs in the Preference set of responsibilities. The tests and the test suite should have the privledge and duty of setting the font preferences to the reset state.
There should also be a #saveCurrent FontPreferences and #restoreFontPreferences which act as a push and pop on the Font testable state. This can be run before and after the test suite to get back the users state.
A first pass could leave the save and restore methods no-ops. We'd still be better off than with the current state of affairs.
----
About your earlier comment. I realized the test was way off base.
My policy when bug tracking is if (when) I find bugs while looking for other things is get them documented and on mantis before going back to my original task. Such was the case here.
Then when time and interest permit come back and add analysis and maybe a fix.
All of the font tests are somewhat state dependent. So I suspect I will be adding more reports rather than focusing on fixing things.
Also, it would be good to get the author to fix his work. And to get others involved in the process of bug tracking and repair.
Yours in curiosity and service, --Jerome Peace |
|
(0012261 - 755 - 879 - 879 - 879 - 879 - 879)
wiz
06-05-08 22:21
edited on: 06-05-08 22:25
|
UpLoading M6705FontTestFix-wiz.1.cs
from the preamble:
"Change Set: FontTestFix-wiz
Date: 5 June 2008
Author: (wiz) Jerome Peace
wiz 6/5/2008 18:10
This corrects the symtom mentioned in
M6705.
Problem was that the accessor for derivativeFonts gives a list of fonts but because it removes empty elements cannot be relied upon as an array indexed by emphasis.
So we weaken the test to check that the derivative font is identity included in the list rather than at the exact spot it is expected to be.
I also wrote a second test to do the check for the emphasized font after the reset. This makes a first pass error out of the other tests second pass error. To make this test work I had to use the same weakened condition.
"
|
|
(0012262 - 85 - 117 - 117 - 117 - 117 - 117)
wiz
06-05-08 22:23
|
"fix begin"
Installer mantis bug: 6705 fix: 'M6705FontTestFix-wiz.1.cs' .
"fix end" |
|
(0013994 - 45 - 45 - 45 - 45 - 45 - 45)
nicolas cellier
12-17-10 23:03
|
Can't remember when it was fixed, but it was. |
|