And still, one thing that is no obvious with split interdependency, is that the tests testing the two sides are inter-dependent. How do we express this interdependency? If we don't that let us free to modify one single side and break the contract. Nicolas Le 25 février 2012 15:37, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> a écrit :
Ah yes, that was me, since one feature of the Compiler is to insert notifications into source text editor, this has to be tested. - does the Compiler insert the right notification ? - does it insert the notification at the right place ?
As soon as TextMorph and SmalltalkEditor provide a hook for evaluating/compiling source, and a callback for notifications they become inter-dependent. And we'd better write a test documenting the interdependency.
Of course, we can replace the TextEditor with a mock up, like the one in CompilerTest and split the interdependency in two parts. And then assert on the notifier side that: - the compiler emit notifications with 1-based insertion point indices (1 means insert before first character) (Though I don't like the hardcoded positions in CompilerTest, it's not readble, but could be changed).
Right now, the notified side of interdependency is missing, I see no test asserting that: - SmalltalkEditor notify:at:in: insert notifications with 1-based indices
If I change TextEditor insertion points to be 0-based, this will get unnoticed...
And this decomposition being less immediate than my simple tests, we forgot one feature which is to evaluate a sub-selection. This feature makes the interdependency much more complex - is notifier aware of sub-selection location ? If yes, does it provide a location related to beginning of sub-selection of beginning of text ? - same for the notified side...
I'm sure that decomposing the dependency explicitly with tests would help reducing the complexity... That was one of my questions: - what if we change usage of ReadWriteStream on:from:to: in selectionAsStream?
Since I wrote tests in Squeak and simply ported to Pharo, and since I missed CompilerTest (which is in Test-Compiler not in CompilerTests, why the hell having two packages?), we could push refactoring further. I made a class for testing a pair (Compiler notifying a SmalltalkEditor), and a subclass for testing another pair (Compiler notifying nil), but I think Pharo has very new capabilities, like working with console like I/O, so this is another variant to test, and maybe in this case splitting interdependency in two is not an over-engineered feature anymore ;)
Nicolas
Le 25 février 2012 14:23, Pavel Krivanek <pavel.krivanek@gmail.com> a écrit :
Great, The basic Pharo Kernel Jobs can be build again! Leaving aside the fact that number of failing tests significantly increased because someone made compiler tests dependent on TextMorph (setUpForErrorsIn:) :-)
-- Pavel
On Sat, Feb 25, 2012 at 8:43 AM, Marcus Denker <marcus.denker@inria.fr> wrote:
14356 -----
Issue 5376:   AbstractTool fix removeMethods method     http://code.google.com/p/pharo/issues/detail?id=5376
Issue 5371: Â Â XTableForUnicodeFont is not used anywhere (and an internal class) Â Â Â Â http://code.google.com/p/pharo/issues/detail?id=5371
Issue 5372:   Remove old Mac font reading in StrikeFont     http://code.google.com/p/pharo/issues/detail?id=5372
Issue 5370:   AppRegistry class >> #default: dependent on Morphic     http://code.google.com/p/pharo/issues/detail?id=5370
-- Marcus Denker -- http://marcusdenker.de