Calling Emil (again)...
I've become a bit obsessed with your Gibberish slick which logs an NPE and yet completes successfully. It keeps catching me out because I watch the emerging log and think "oh no! I've broken something!", only to check the test reports and see that every test was successful.
Its a bit like the little boy crying wolf... if tests log failures and still work... and other tests are turned off because they don't work... then how can we trust our tests?
Please don't think I'm criticising. I know you agree with me but you can't afford to be too perfectionist at this stage of the project. However, if you can help me out with the high-level design, I'll try to take care of some of the details.
Your failing-but-successful test is exposing a bug in MclStorageManager.createProtoContactGroupNode(), but surprisingly this bug is not exposed in the ContactList slick.
protoGroup.getParentContactGroup() is legitimately returning null because the given group is the root element, but the caller assumes it will always receive an valid instance.
So.... let me have your first impressions and I'll try to sort it out myself quickly.
1. It seems reasonable to return a null parent when the group is the root, so I currently believe the method comment block. Do you agree this is topologically correct according to the original design?
2. If so, then the calling logic should test for a null parent before trying to invoke an instance method on null. That needs recoding defensively.
3. Does it seem reasonable that the ContactList slick should test this boundary case? If yes, then we need a new test here, which must also fail, before the bug is fixed.
4. The Gibberish test should not be returning success when it is NOT deliberately catching the NPE and claiming "expected behaviour". I need to make this test report a failure - then it will be successful after the bug is fixed.
Thanks for your time...