Implement Unit Testing Mechanism #173
Conversation
jleandroperez
commented
Nov 12, 2013
jleandroperez
commented
- Updated our Integration Tests to use Xcode 5 testing framework.
- Implemented Mock-Objects mechanism.
- Implemented Remote-Debugging Unit Test.
Conflicts: Simperium/SPWebSocketInterface.m Simperium/Simperium.m
…WebSockets mock objects and mock-mechanism. Adding Unit Tests.
| @@ -597,10 +597,13 @@ - (NSArray*)exportPendingChanges { | |||
| // This routine shall be used for debugging purposes! | |||
| NSMutableArray* pendings = [NSMutableArray array]; | |||
| for(NSDictionary* change in changesPending.allValues) { | |||
|
|
|||
| NSString* startVersion = [change[CH_START_VERSION] copy] ?: @""; | |||
There was a problem hiding this comment.
Tricks like this in a protocol parser make me a little uncomfortable. Should CH_START_VERSION always be present in a change? What happens if it's not? Should we at least log an error, and maybe bail?
There was a problem hiding this comment.
As far as i can tell, under real circumstances that should always be present. However, while running unit tests (with Simperium 100% disconnected from the backend) i got a couple scenarios in which it's set to nil, and the unit test itself can crash (due to a nil insertion inside a NSDictionary).
There was a problem hiding this comment.
Just checked Beau's specs:
sv : the source version for the entity the change applies to. Note: not sent for new objects.
So it's okay, actually, not to send it. I'll update the code so the export call simply doesn't add that field, instead of adding a blank.
|
|
||
| @implementation MockSimperium | ||
|
|
||
| +(MockSimperium*)newMockSimperium |
There was a problem hiding this comment.
Naming convention for a class method that returns an instance should probably be mockSimperium instead of newMockSimperium.
There was a problem hiding this comment.
Under the 'old school' memory management guidelines, if a method returns a non-autorelease instance, the name should include 'new' keyword. With ARC, that's not the case anymore right? (haven't re checked Apple's guidelines).
There was a problem hiding this comment.
Couldn't find anything official here. But yeah, new feels old school. =)
There was a problem hiding this comment.
After verifying that under ARC, the lack of the 'new' keyword doesn't make any difference, we're officially nuking the new keyword.
Ref: 8a5e053
Thanks again!
|
Great cleanup, cool tests. This'll come in very handy. |
|
Thank you!. Let's add tests for everything (Gradually speaking!). |
Implement Unit Testing Mechanism
