2017-06-04 21:28 GMT+02:00 Cyril Ferlicot D. <cyril.ferlicot@gmail.com>:
Le 04/06/2017 �� 21:26, Cyril Ferlicot D. a ��crit :
> For this problem I would like to see all the smalltalks using SUnit
> change this behaviour. I really do not want to break compatibility and I
> think this change is important for the usability of SUnit. I don't have
> much contact with other smalltalk communities but I would be happy if we
> can seek a consensus.
>
> If you have an idea of the best way to communicate with all the
> communities on this subject I would like to hear it.
>

Maybe we can talk about it at ESUG?


It can be interesting to measure instead of throwing 99% figures like I did ;)
Here is a small snippet and result in Squeak trunk:

| testAll testLocal superIsAbstractOrTestSelectorIsEmpty superclassIsAbstract selectorsIsEmpty selectorsIsEmptyButNotAbstract |
testAll := Set new.
testLocal := Set new.
superIsAbstractOrTestSelectorIsEmpty := Set new.
superclassIsAbstract := Set new.
selectorsIsEmpty := Set new.
selectorsIsEmptyButNotAbstract := Set new.
TestCase allSubclassesDo: [:c |
������ c superclass = TestCase
������ ������ ifFalse: [
������ ������ ������ (c superclass isAbstract or: [c testSelectors isEmpty])
������ ������ ������ ������ ifTrue: [superIsAbstractOrTestSelectorIsEmpty add: c].
������ ������ ������ c superclass isAbstract
������ ������ ������ ������ ifTrue: [superclassIsAbstract add: c]
������ ������ ������ ������ ifFalse: [c testSelectors isEmpty ifTrue: [selectorsIsEmptyButNotAbstract add: c]].
������ ������ ������ c testSelectors isEmpty ifTrue: [selectorsIsEmpty add: c].
������ ������ ������ c shouldInheritSelectors
������ ������ ������ ������ ifTrue: [testAll add: c]
������ ������ ������ ������ ifFalse: [testLocal add: c]]].
{testAll size. superIsAbstractOrTestSelectorIsEmpty size. superclassIsAbstract size. selectorsIsEmptyButNotAbstract size. selectorsIsEmpty size. testLocal size}.
��#(143 142 129 13 17 22)

So 143 classes want to inherit
- 129 because superclass isAbstract,
- 13 because testSelectors isEmpty,
- 1 for another reason - override of shouldInheritSelectors, for which we should write a snippet too
22 don't want to inherit

But here are the 22 in Squeak:
(testLocal collect: #name) sorted ->
#(#ChangeHooksTest #ClassTraitTest #MessageTraceTest #MethodHighlightingTests #MorphBugs #MorphicEventDispatcherTests #MorphicEventFilterTests #MorphicEventTests #PolygonMorphTest #PureBehaviorTest #StickynessBugz #SystemChangeErrorHandling #SystemChangeNotifierTest #TestNewParagraphFix #TextAnchorTest #TextEmphasisTest #TextFontChangeTest #TraitCompositionTest #TraitFileOutTest #TraitMethodDescriptionTest #TraitSystemTest #TraitTest)

and their superclass
((testLocal collect: #superclass) collect: #name) sorted ->
#(#ClosureCompilerTest #HashAndEqualsTestCase #MessageSetTest #MorphTest #MorphicUIManagerTest #SystemChangeTestRoot #TestParagraphFix #TraitsTestCase #UserInputEventTests)

Let's see;
- TraitsTestCase is not abstract, but defines no test! So it is kind of abstract.
�� TraitsTestCase subclasses size -> 7, 15 remain out of 22
- same for SystemChangeTestRoot, 4 subclasses, 11 remain out of 22.
- same for UserInputEventTests. 3 subclasses less, 8 remaining out of 22.
- TestNewParagraphFix does redefine all super test as ^super test... A lack of knowledge visibly. 7 remaining out of 22.
- HashAndEqualsTestCase is not abstract, has tests, but the tested prototypes are empty!
�� So it is yet another abstract class in disguise.
�� But this time, some subclasses won't testHash nor testEquality except if they redefine it, is it a bug?
�� That's 3 subclasses with not empty selectors, so 4 remaining out of 22

The others might be correct, so 161 ou of 165 should inherit... Not 99%, only near 98%.

That would be 4 (super)classes requiring implementation of a shouldInheritSelectors ^false.
That's too much, so let's analyze further.

Maybe MethodHighlightingTests does not need to inherit from ClosureCompilerTest...�� (no inst var in super, and no super method used). That 1 less.
MorphBugs and PolygonMorphTest don't need to be a subclass (it's just for classifying). There is no specific setUp, and tests could go upward. That's 2 less.
Same for StickynessBugz. 1 less.

After an easy refactoring, nothing in Squeak trunk would really require a shouldInheritSelectors ^false.
It would be interesting to replay the snippet in Pharo, and in 3rd party applications too.
That's good elements for opening a discussion.


--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Pr��vert 01,
59650 Villeneuve d'ascq France