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 >> >> >
