Spec code completion

[guillep]

Extend rubric and Spec to be able to set the code completion engine from the outside.

• Code completion should be initialized lazily to the global one only.
• Extended Code presenter and adaptor to set it in widget if available in presenter

[myroslavarm]

would be interesting to use this :)

[MarcusDenker]

Ci shows lots of failing spec tests

[guillep]

testInstanceVersusSharedCompletionEngine

	| textEditor currentCompletion |
	[ currentCompletion := RubSmalltalkEditor completionEngineClass.
	textEditor := RubSmalltalkEditor new.
	self assert: textEditor completionEngine class equals: currentCompletion.
	
	RubSmalltalkEditor noCompletion.
	"here we show that an existing editor is not impacted by the shared behavior"
	self assert: textEditor completionEngine class equals: currentCompletion.
	textEditor := RubSmalltalkEditor new.
	self assert: textEditor completionEngine isNil.
	
	RubSmalltalkEditor completionEngineClass: CompletionEngine.
	self assert: textEditor completionEngine isNil.
	
	"but now when we create a new one it gets the correct engine"
	textEditor := RubSmalltalkEditor new.
	self assert: textEditor completionEngine class equals: CompletionEngine.
	
	
	] ensure: [ RubSmalltalkEditor completionEngineClass: currentCompletion ]

This test is breaking, but I find it super brittle...

[Ducasse]

I will ask you why you find it brittle.

[Ducasse]

Guille apparently the methods are not well categorized.

[guillep]

I will ask you why you find it brittle.

Well, I particularly do not like that the code depends on a singleton class like that.
But being more specific => I ran that code, and suddenly my system was broken. So I moved the initial state and last part of the test to setUp and tearDown like it should be. Even if the test fails, like that the test should ensure my system stays in a correct state.

Then, I find overly complex that the test does:

• side effect 1
• assert 1
• side effect 2 that partially intersects with side effect 1
• assert 1 should not be valid anymore
• and now assert 2
  ...

I do not like that the test is a succession of side-effect dependencies with strange semantics.
Evolving that test is a pain :(. If I change side effect 1, it breaks some parts of the test, some others not, it's difficult to understand why...

Are those assert independent? Are they dependent on each other? If they are independent, why not using different tests?

[Ducasse]

Ok I understand some points. But the ensure section was not making sure that the state of the system was ok?
I mean is it better to have setup and teardown vs. ensure?

[Ducasse]

I agree with the sequence and overlapping part :)

[Ducasse]

Now I asked because I could have written such a test :)

[guillep]

I mean is it better to have setup and teardown vs. ensure?

I know you're right about that :).
I'm just a bit mad about the state of things so I over-complain ^^.

Now, the problem with this global state I think is this:

• an exception is raised in the middle of this test
• the execution is suspended in the middle of the block, but the ensure is not executed yet
  => otherwise you would be debugging something different from what caused the exception
• the debugger opens, but boom => code completion is maybe in a broken state!

Just a singleton problem :)

[guillep]

Now, I added two tests that show how we can modify the code completion engine per instance. The test is not so interesting, it just shows we can ignore the global variable :).

[Ducasse]

I see I like the analysis. Indeed I did not think about the debugger before the ensure.