Please refer to the attached changesets for the review. I split up the original change set into two parts, improved the documentation, and fixed the handling of trait wrappers.<br>
<br>
I'm also attaching a third changeset that you can use to explore the behavior of the TestRunner for coverage testing with traits.<br>
<br>
If you don't have any objections, I am planning to merge this next week. :-)<br>
<br>
Best,<br>
Christoph<br>
<br>
<b>=============== Summary {Traits-eventualUsers.3.cs} ===============</b><br>
<br>
Change Set: Traits-eventualUsers<br>
Date: 29 September 2022<br>
Author: Christoph Thiede<br>
<br>
Adds new accessors to Traits for accessing all direct and indirect users of a trait or a trait's method:<br>
<br>
* TraitDescription allUsers, TraitDescription eventualUsers<br>
* CompiledMethod eventualUsers, CompiledMethod eventualOriginalTraitMethod<br>
* MethodReference eventualUsers, MethodReference eventualUserReferences<br>
<br>
Also includes tests.<br>
<br>
<b>=============== Diff ===============</b><br>
<br>
<b>CompiledMethod>>eventualOriginalTraitMethod {*Traits-NanoKernel} · ct 9/29/2022 10:53</b><br>
<font color="#FF0000">+ eventualOriginalTraitMethod<br>
+ "Answer the eventual original trait method for the receiver that is not a trait method itself, or nil if none."<br>
+ ^ self originalTraitMethod ifNotNil: [:method |<br>
+ method eventualOriginalTraitMethod ifNil: [method]]</font><br>
<br>
<b>CompiledMethod>>eventualUsers {*Traits-NanoKernel} · TraitTest 9/9/2022 23:37</b><br>
<font color="#FF0000">+ eventualUsers<br>
+ <br>
+ ^ self methodClass eventualUsersForSelector: self selector</font><br>
<br>
<b>CompiledMethod>>isTraitMethod {*Traits-testing} · ct 9/22/2022 16:01 (changed)</b><br>
isTraitMethod<br>
<font color="#FF0000">+ "Answer a Boolean indicating whether the receiver is derived (i.e., copied) from a Trait.<br>
+ <br>
+ Note: For testing whether a method *belongs* to a trait, send [aCompiledMethod methodClass isTrait] instead."<br>
</font><br>
^ self originalTraitMethod notNil<br>
<br>
<b>MethodReference>>eventualUserReferences {*Traits-NanoKernel} · ct 9/9/2022 22:12</b><br>
<font color="#FF0000">+ eventualUserReferences<br>
+ <br>
+ ^ self eventualUsers collect: [:classDescription |<br>
+ (classDescription >> self selector) methodReference]</font><br>
<br>
<b>MethodReference>>eventualUsers {*Traits-NanoKernel} · ct 9/9/2022 20:11</b><br>
<font color="#FF0000">+ eventualUsers<br>
+ <br>
+ ^ self compiledMethod eventualUsers</font><br>
<br>
<b>TraitDescription>>allUsers {accessing} · ct 9/9/2022 23:06</b><br>
<font color="#FF0000">+ allUsers<br>
+ "Answer all ClassDescriptions that directly or indirectly (i.e., through a path of other traits) use the receiver."<br>
+ <br>
+ ^ self users gather: [:classDescription |<br>
+ classDescription isTrait<br>
+ ifTrue: [classDescription allUsers copyWithFirst: classDescription]<br>
+ ifFalse: [{classDescription}]]</font><br>
<br>
<b>TraitDescription>>eventualUsers {accessing} · ct 9/9/2022 23:07</b><br>
<font color="#FF0000">+ eventualUsers<br>
+ "Answer all non-trait ClassDescriptions that directly or indirectly (i.e., through a path of other traits) use the receiver."<br>
+ <br>
+ ^ self users gather: [:classDescription |<br>
+ classDescription isTrait<br>
+ ifTrue: [classDescription eventualUsers]<br>
+ ifFalse: [{classDescription}]]</font><br>
<br>
<b>TraitDescription>>eventualUsersForSelector: {accessing} · ct 9/29/2022 10:52</b><br>
<font color="#FF0000">+ eventualUsersForSelector: aSymbol<br>
+ "Answer all eventual users that use the receiver's implementation for aSymbol, i.e., exclude all users that redefine that message."<br>
+ <br>
+ | originalMethod |<br>
+ originalMethod := self >> aSymbol.<br>
+ ^ self eventualUsers select: [:classDescription |<br>
+ | userMethod |<br>
+ (userMethod := classDescription compiledMethodAt: aSymbol ifAbsent: []) notNil and:<br>
+ [userMethod eventualOriginalTraitMethod == originalMethod]]</font><br>
<br>
<b>TraitTest>>testAllUsers {testing} · TraitTest 9/9/2022 23:17</b><br>
<font color="#FF0000">+ testAllUsers<br>
+ self assert: self t1 allUsers size = 4.<br>
+ self assert: (self t1 allUsers includesAllOf: {self t4. self t5. self c2. self t6 }).<br>
+ self assert: self t3 allUsers isEmpty.<br>
+ self assert: self t5 allUsers size = 1.<br>
+ self assert: self t5 allUsers anyOne = self c2.<br>
+ self c2 uses: self t1 + self t5.<br>
+ self assert: self t5 allUsers size = 1.<br>
+ self assert: self t5 allUsers anyOne = self c2.<br>
+ self c2 uses: self t2 asTraitComposition.<br>
+ self assert: self t5 allUsers isEmpty</font><br>
<br>
<b>TraitTest>>testEventualUserReferences {testing} · TraitTest 9/9/2022 23:41</b><br>
<font color="#FF0000">+ testEventualUserReferences<br>
+ self assert: (self t1 >> #m11) methodReference eventualUserReferences isEmpty.<br>
+ self assert: (self t1 >> #m12) methodReference eventualUserReferences isEmpty.<br>
+ self assert: (self t2 >> #m21) methodReference eventualUserReferences isEmpty.<br>
+ self assert: (self t3 >> #m31) methodReference eventualUserReferences isEmpty.<br>
+ self assert: (self t5 >> #m11) methodReference eventualUserReferences isEmpty.<br>
+ self assert: (self t5 >> #m12) methodReference eventualUserReferences size = 1.<br>
+ self assert: (self t5 >> #m12) methodReference eventualUserReferences anyOne = (self c2 >> #m12) methodReference.<br>
+ self assert: (self t5 >> #m21) methodReference eventualUserReferences size = 1.<br>
+ self assert: (self t5 >> #m21) methodReference eventualUserReferences anyOne = (self c2 >> #m21) methodReference.<br>
+ self assert: (self t5 >> #m51) methodReference eventualUserReferences size = 1.<br>
+ self assert: (self t5 >> #m51) methodReference eventualUserReferences anyOne = (self c2 >> #m51) methodReference.</font><br>
<br>
<b>TraitTest>>testEventualUsers {testing} · TraitTest 9/9/2022 23:18</b><br>
<font color="#FF0000">+ testEventualUsers<br>
+ self assert: self t5 eventualUsers size = 1.<br>
+ self assert: self t5 eventualUsers anyOne = self c2.<br>
+ self assert: self t1 eventualUsers size = 1.<br>
+ self assert: self t1 eventualUsers anyOne = self c2.<br>
+ self assert: self t3 eventualUsers isEmpty.</font><br>
<br>
<br>
<b>=============== Summary {traits.2.cs} ===============</b><br>
<br>
Change Set: SUnit coverage for traits<br>
Date: 29 September 2022<br>
Author: Christoph Thiede<br>
<br>
Adds proper support for Traits to SUnit's test coverage. As the methods from a trait are copied to all its eventual users, all copies should be treated as a single coverable item to the TestRunner, just like a method from a superclass that is inherited by multiple subclasses.<br>
<br>
Implementation:<br>
* Exclude derived trait methods in TestRunner>>#addMethodsUnderTestIn:to:<br>
* Include wrappers for all eventual users of each original trait method in TestRunner>>#collectCoverageFor: and synchronize them with their original method in TestCoverage>>#mark<br>
* No dependency on Traits is introduced, SUnit can still be loaded and used if Traits is not loaded<br>
<br>
Depends on Traits-eventualUsers.<br>
<br>
<b>=============== Diff ===============</b><br>
<br>
<b>CompiledMethod>>sunitIsTraitMethod {*SUnitGUI-testing} · ct 9/22/2022 16:16</b><br>
<font color="#FF0000">+ sunitIsTraitMethod<br>
+ "Wrapper for #isTraitMethod to avoid dependency on Traits."<br>
+ <br>
+ ^ (self respondsTo: #isTraitMethod)<br>
+ and: [self isTraitMethod]</font><br>
<br>
<b>Object>>sunitIsTrait {*SUnitGUI-testing} · ct 9/22/2022 16:18</b><br>
<font color="#FF0000">+ sunitIsTrait<br>
+ "Wrapper for #isTrait to avoid dependency on Traits."<br>
+ <br>
+ ^ (self respondsTo: #isTrait)<br>
+ and: [self isTrait]</font><br>
<br>
<b>TestCoverage>>mark {private} · ct 9/29/2022 11:03 (changed)</b><br>
mark<br>
<s><font color="#0000FF">- hasRun := true<br>
</font></s><font color="#FF0000">+ hasRun := true.<br>
+ <br>
+ self sunitIsTraitMethod ifTrue:<br>
+ ["update original (shared) trait method"<br>
+ | original |<br>
+ original := self eventualOriginalTraitMethod methodReference compiledMethod.<br>
+ (original respondsTo: #mark) ifTrue:<br>
+ [original mark; uninstall]].</font><br>
<br>
<b>TestCoverage>>respondsTo: {private} · ct 9/9/2022 22:22</b><br>
<font color="#FF0000">+ respondsTo: aMessage<br>
+ <br>
+ ^ (self class canUnderstand: aMessage)<br>
+ or: [method respondsTo: aMessage]</font><br>
<br>
<b>TestRunner>>addMethodsUnderTestIn:to: {actions} · ct 9/22/2022 16:17 (changed)</b><br>
addMethodsUnderTestIn: packages to: methods <br>
packages<br>
do: [:package | package isNil<br>
ifFalse: [package methods<br>
do: [:method | ((#(#packageNamesUnderTest #classNamesNotUnderTest ) includes: method methodSymbol)<br>
<s><font color="#0000FF">- or: [method compiledMethod isAbstract<br>
- or: [method compiledMethod hasLiteral: #ignoreForCoverage]])<br>
</font></s><font color="#FF0000">+ or: [method compiledMethod isAbstract]<br>
+ or: [method compiledMethod sunitIsTraitMethod "trait methods are derived from a method in a trait - only the original method is relevant for us"]<br>
+ or: [method compiledMethod hasLiteral: #ignoreForCoverage])<br>
</font> ifFalse: [methods add: method]]]]<br>
<br>
<b>TestRunner>>collectCoverageFor: {actions} · ct 9/22/2022 16:09 (changed)</b><br>
collectCoverageFor: methods<br>
<s><font color="#0000FF">- | wrappers suite |<br>
- wrappers := methods collect: [ :each | TestCoverage on: each ].<br>
</font></s><font color="#FF0000">+ | allMethods wrappers suite |<br>
+ "Methods in traits are copied to their users. Wrap these copies as well to catch all evaluations of the original trait method."<br>
+ allMethods := methods<br>
+ , ((methods select: [ :each | each actualClass isTrait ])<br>
+ gather: [ :each | each eventualUserReferences ]).<br>
+ wrappers := allMethods collect: [ :each | TestCoverage on: each ].<br>
</font> suite := self<br>
reset;<br>
suiteAll.<br>
<br>
[ wrappers do: [ :each | each install ].<br>
<s><font color="#0000FF">- [ self runSuite: suite ] ensure: [ wrappers do: [ :each | each uninstall ] ] ] valueUnpreemptively.<br>
- wrappers := wrappers reject: [ :each | each hasRun ].<br>
</font></s><font color="#FF0000">+ [ self runSuite: suite ] ensure: [ wrappers do: [ :each | each uninstall ] ] ]<br>
+ valueUnpreemptively.<br>
+ wrappers := wrappers reject: [ :each | each hasRun<br>
+ or: [ each isTraitMethod "trait methods' wrappers update their original method wrapper automatically, see TestCoverage>>#mark" ] ].<br>
+ <br>
</font> wrappers isEmpty <br>
ifTrue: <br>
[ UIManager default inform: 'Congratulations. Your tests cover all code under analysis.' ]<br>
ifFalse: <br>
[ ToolSet <br>
browseMessageSet: (wrappers collect: [ :each | each reference ])<br>
name: 'Not Covered Code (' , (100 - (100 * wrappers size // methods size)) printString , '% Code Coverage)'<br>
autoSelect: nil ].<br>
self saveResultInHistory<br>
<br>
<br>
<font color="#808080">---<br>
</font><font color="#808080"><i>Sent from </i></font><font color="#808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><font color="#808080">Squeak Inbox Talk</font></u></a></i></font><br>
<br>
On 2022-09-09T23:53:26+02:00, christoph.thiede@student.hpi.uni-potsdam.de wrote:<br>
<br>
> =============== Summary ===============<br>
> <br>
> Change Set:????????TestCoverage-traits<br>
> Date:????????????9 September 2022<br>
> Author:????????????Christoph Thiede<br>
> <br>
> Adds proper support for trait methods to SUnit's test coverage collection.<br>
> <br>
> Details:<br>
> * Adds #eventualUsers/#allUsers protocol on TraitDescription and MethodReference for browing users of a trait and its methods.<br>
> * Tests the new traits protocol.<br>
> * TestRunner: Excludes trait methods (i.e., methods copied from a trait) from test coverage methods. Installs extra wrappers for all eventual users of each method defined in a trait and synchronizes them via TestCoverage>>#mark.<br>
> * Implements proper #repondsTo: on TestCoverage.<br>
> <br>
> =============== Diff ===============<br>
> <br>
> CompiledMethod>>eventualUsers {*Traits-NanoKernel} ? TraitTest 9/9/2022 23:37<br>
> + eventualUsers<br>
> + <br>
> + ????^ self methodClass eventualUsersForSelector: self selector<br>
> <br>
> MethodReference>>eventualUserReferences {*Traits-NanoKernel} ? ct 9/9/2022 22:12<br>
> + eventualUserReferences<br>
> + <br>
> + ????^ self eventualUsers collect: [:classDescription |<br>
> + ????????(classDescription >> self selector) methodReference]<br>
> <br>
> MethodReference>>eventualUsers {*Traits-NanoKernel} ? ct 9/9/2022 20:11<br>
> + eventualUsers<br>
> + <br>
> + ????^ self compiledMethod eventualUsers<br>
> <br>
> TestCoverage>>mark {private} ? ct 9/9/2022 22:22 (changed)<br>
> mark<br>
> - ????hasRun := true<br>
> + ????hasRun := true.<br>
> + ????<br>
> + ????self isTraitMethod ifTrue:<br>
> + ????????[| original |<br>
> + ????????original := self originalTraitMethod methodReference compiledMethod.<br>
> + ????????(original respondsTo: #mark) ifTrue:<br>
> + ????????????[original mark; uninstall]].<br>
> <br>
> TestCoverage>>respondsTo: {private} ? ct 9/9/2022 22:22<br>
> + respondsTo: aMessage<br>
> + <br>
> + ????^ (self class canUnderstand: aMessage)<br>
> + ????????or: [method respondsTo: aMessage]<br>
> <br>
> TestRunner>>addMethodsUnderTestIn:to: {actions} ? ct 9/9/2022 22:16 (changed)<br>
> addMethodsUnderTestIn: packages to: methods <br>
> ????packages<br>
> ????????do: [:package | package isNil<br>
> ????????????????ifFalse: [package methods<br>
> ????????????????????????do: [:method | ((#(#packageNamesUnderTest #classNamesNotUnderTest ) includes: method methodSymbol)<br>
> - ????????????????????????????????????or: [method compiledMethod isAbstract<br>
> - ????????????????????????????????????????????or: [method compiledMethod hasLiteral: #ignoreForCoverage]])<br>
> + ????????????????????????????????????or: [method compiledMethod isAbstract]<br>
> + ????????????????????????????????????or: [method compiledMethod isTraitMethod]<br>
> + ????????????????????????????????????or: [method compiledMethod hasLiteral: #ignoreForCoverage])<br>
> ????????????????????????????????ifFalse: [methods add: method]]]]<br>
> <br>
> TestRunner>>collectCoverageFor: {actions} ? ct 9/9/2022 23:00 (changed)<br>
> collectCoverageFor: methods<br>
> - ????| wrappers suite |<br>
> - ????wrappers := methods collect: [ :each | TestCoverage on: each ].<br>
> + ????| methodsWithTraits wrappers suite |<br>
> + ????methodsWithTraits := methods , ((methods select: [ :each | each actualClass isTrait ]) gather: [ :each | each eventualUserReferences ]).<br>
> + ????wrappers := methodsWithTraits collect: [ :each | TestCoverage on: each ].<br>
> ????suite := self<br>
> ????????reset;<br>
> ????????suiteAll.<br>
> ????<br>
> ????[ wrappers do: [ :each | each install ].<br>
> ????[ self runSuite: suite ] ensure: [ wrappers do: [ :each | each uninstall ] ] ] valueUnpreemptively.<br>
> - ????wrappers := wrappers reject: [ :each | each hasRun ].<br>
> + ????wrappers := wrappers reject: [ :each | each hasRun or: [ each isTraitMethod ] ].<br>
> + ????<br>
> ????wrappers isEmpty <br>
> ????????ifTrue: <br>
> ????????????[ UIManager default inform: 'Congratulations. Your tests cover all code under analysis.' ]<br>
> ????????ifFalse: <br>
> ????????????[ ToolSet <br>
> ????????????????browseMessageSet: (wrappers collect: [ :each | each reference ])<br>
> ????????????????name: 'Not Covered Code (' , (100 - (100 * wrappers size // methods size)) printString , '% Code Coverage)'<br>
> ????????????????autoSelect: nil ].<br>
> ????self saveResultInHistory<br>
> <br>
> TraitDescription>>allUsers {accessing} ? ct 9/9/2022 23:06<br>
> + allUsers<br>
> + ????"Answer all ClassDescriptions that directly or indirectly (i.e., through a path of other traits) use the receiver."<br>
> + <br>
> + ????^ self users gather: [:classDescription |<br>
> + ????????classDescription isTrait<br>
> + ????????????ifTrue: [classDescription allUsers copyWithFirst: classDescription]<br>
> + ????????????ifFalse: [{classDescription}]]<br>
> <br>
> TraitDescription>>eventualUsers {accessing} ? ct 9/9/2022 23:07<br>
> + eventualUsers<br>
> + ????"Answer all non-trait ClassDescriptions that directly or indirectly (i.e., through a path of other traits) use the receiver."<br>
> + <br>
> + ????^ self users gather: [:classDescription |<br>
> + ????????classDescription isTrait<br>
> + ????????????ifTrue: [classDescription eventualUsers]<br>
> + ????????????ifFalse: [{classDescription}]]<br>
> <br>
> TraitDescription>>eventualUsersForSelector: {accessing} ? TraitTest 9/9/2022 23:26<br>
> + eventualUsersForSelector: aSymbol<br>
> + ????"Answer all eventual users that use the implementation of for aSymbol of the receiver, i.e., exclude all users that override this message."<br>
> + <br>
> + ????| originalMethod |<br>
> + ????originalMethod := self >> aSymbol.<br>
> + ????^ self eventualUsers select: [:classDescription |<br>
> + ????????| userMethod |<br>
> + ????????(userMethod := classDescription compiledMethodAt: aSymbol ifAbsent: []) notNil and:<br>
> + ????????????[userMethod originalTraitMethod == originalMethod]]<br>
> <br>
> TraitTest>>testAllUsers {testing} ? TraitTest 9/9/2022 23:17<br>
> + testAllUsers<br>
> + ????self assert: self t1 allUsers size = 4.<br>
> + ????self assert: (self t1 allUsers includesAllOf: {self t4. self t5. self c2. self t6 }).<br>
> + ????self assert: self t3 allUsers isEmpty.<br>
> + ????self assert: self t5 allUsers size = 1.<br>
> + ????self assert: self t5 allUsers anyOne = self c2.<br>
> + ????self c2 uses: self t1 + self t5.<br>
> + ????self assert: self t5 allUsers size = 1.<br>
> + ????self assert: self t5 allUsers anyOne = self c2.<br>
> + ????self c2 uses: self t2 asTraitComposition.<br>
> + ????self assert: self t5 allUsers isEmpty<br>
> <br>
> TraitTest>>testEventualUserReferences {testing} ? TraitTest 9/9/2022 23:41<br>
> + testEventualUserReferences<br>
> + ????self assert: (self t1 >> #m11) methodReference eventualUserReferences isEmpty.<br>
> + ????self assert: (self t1 >> #m12) methodReference eventualUserReferences isEmpty.<br>
> + ????self assert: (self t2 >> #m21) methodReference eventualUserReferences isEmpty.<br>
> + ????self assert: (self t3 >> #m31) methodReference eventualUserReferences isEmpty.<br>
> + ????self assert: (self t5 >> #m11) methodReference eventualUserReferences isEmpty.<br>
> + ????self assert: (self t5 >> #m12) methodReference eventualUserReferences size = 1.<br>
> + ????self assert: (self t5 >> #m12) methodReference eventualUserReferences anyOne = (self c2 >> #m12) methodReference.<br>
> + ????self assert: (self t5 >> #m21) methodReference eventualUserReferences size = 1.<br>
> + ????self assert: (self t5 >> #m21) methodReference eventualUserReferences anyOne = (self c2 >> #m21) methodReference.<br>
> + ????self assert: (self t5 >> #m51) methodReference eventualUserReferences size = 1.<br>
> + ????self assert: (self t5 >> #m51) methodReference eventualUserReferences anyOne = (self c2 >> #m51) methodReference.<br>
> <br>
> TraitTest>>testEventualUsers {testing} ? TraitTest 9/9/2022 23:18<br>
> + testEventualUsers<br>
> + ????self assert: self t5 eventualUsers size = 1.<br>
> + ????self assert: self t5 eventualUsers anyOne = self c2.<br>
> + ????self assert: self t1 eventualUsers size = 1.<br>
> + ????self assert: self t1 eventualUsers anyOne = self c2.<br>
> + ????self assert: self t3 eventualUsers isEmpty.<br>
> <br>
> ["TestCoverage-traits.1.cs"]<br>
> <br>
> ---<br>
> Sent from Squeak Inbox Talk<br>
> -------------- next part --------------<br>
> An HTML attachment was scrubbed...<br>
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220909/f231201b/attachment.html><br>
> -------------- next part --------------<br>
> A non-text attachment was scrubbed...<br>
> Name: TestCoverage-traits.1.cs<br>
> Type: application/octet-stream<br>
> Size: 6396 bytes<br>
> Desc: not available<br>
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220909/f231201b/attachment.obj><br>
> <br>
> <br>
["Traits-eventualUsers.3.cs"]<br>
["SUnit coverage for traits.2.cs"]<br>
["Traits-eventualUsers.3.cs"]<br>
["SUnit coverage for traits.2.cs"]<br>
["TraitCoverageExample.3.cs"]