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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >> 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 >>> >>> >>
