Resolving MCSerializationTest>>testMcdSerialization a little rant

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Resolving MCSerializationTest>>testMcdSerialization a little rant

Nicolas Cellier
The test relies on

mockDiffyVersion
    | repos workingCopy base next |
    repos := MCDictionaryRepository new.
    workingCopy := MCWorkingCopy forPackage: self mockPackage.
    workingCopy repositoryGroup addRepository: repos.
    MCRepositoryGroup default removeRepository: repos.
    base := self mockVersion.
    repos storeVersion: base.
    self change: #a toReturn: 'a2'.
    next := self mockVersionWithAncestor: base.
    ^ next asDiffAgainst: base   

As I said in another thread, this test is not working as intended.
Despite the intention to change #a toReturn: 'a2', and thus have a diff,
the base version and next version share the same snapshot.
Thus there is no change detected.
Thus there is a [Warning signal: 'About to serialize an empty package.'].

The first reason why this is not working is because #mockVersion and #mockVersionWithAncestor both send #mockSnapshot.
But #mockSnapshot is not dynamic, it's rather a fixed ressource (see MCSnaphsotResource>>snapshot, setUp, takeSnapshot).
So the first thing is to change mockVersionWithAncestor: to use (self mockPackage snapshot instead) of (self mockSnapshot) - oh yeah that's too obvious, isn't it?

But that's not enough... What is exactly the state retained in MCSnaphsotResource?
Apparently #a already returns 'a2' in the snapshot, probably because this is what is stored in the Tests package.
And this is what is stored in Tests package probably because MCSerializationTest never invoke #restoreMocks. Some SUnit do this in the tearDown, but it looks like random to me...

So we have many tests messing with a global state... This global state creates many interactions.
As someone who did not write these tests, I'd say it's impossible to get a clear picture ot all those interactions, even after a few hours of browsing and debugging.

So who should send restoreMocks?
There is not a comment anywhere explaining what the responsibilities and the contracts are.
It's very good to have self explaining code, but here, having no guideline for such a complex beast, I personnally see it as self fooling code.