Skip to content
This repository has been archived by the owner on Jan 17, 2019. It is now read-only.

Use more appropriate asserts #259

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Use more appropriate asserts #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb26062
Use more appropriate asserts
cerihughes Dec 22, 2016
fd6d6ed
Check for the same instance.
cerihughes Dec 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions tests/HUBActionRegistryTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ - (void)testRegisteringFactoryAndCreatingAction
HUBActionMock * const action = [[HUBActionMock alloc] initWithBlock:nil];
HUBActionFactoryMock * const factory = [[HUBActionFactoryMock alloc] initWithActions:@{actionIdentifier.namePart: action}];
[self.actionRegistry registerActionFactory:factory forNamespace:actionIdentifier.namespacePart];

XCTAssertEqual([self.actionRegistry createCustomActionForIdentifier:actionIdentifier], action);

// These should be the same instance.
XCTAssertTrue([self.actionRegistry createCustomActionForIdentifier:actionIdentifier] == action);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really fail to understand why this is better than XCTAssertEqual which is meant for exactly these type of comparisons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Ceri; XCTAssertEqual is a smell for non-scalar types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's because in a test, if I look at a line that says XCTAssertEqual(objA, obj2), I immediately think "do we really want to check equality here, or is this a typo".

Admittedly XCTAssertTrue(obj1 == obj2) doesn't do a great deal to fix that ambiguity, which is why I like Hamcrest's approach of providing a "same instance" matcher for objects so that it's obvious by looking at it whether you want to check for equality or instance equivalence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment is the most useful thing here. I'll revert the assert and keep the comment.

Copy link
Contributor

@rastersize rastersize Jan 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding a description to the assert? That is:

XCTAssertTrue([self.actionRegistry createCustomActionForIdentifier:actionIdentifier] == action,
              @"The action should be the same instance as the factory mock vends");

That way the comment will be directly attached to the assert. Making sure it also shows up when the test fails.

}

- (void)testRegisteringAlreadyRegisteredFactoryThrows
Expand Down
2 changes: 1 addition & 1 deletion tests/HUBComponentGestureRecognizerTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ - (void)tearDown

- (void)testGestureRecognizerAddedToView
{
XCTAssertEqual(self.gestureRecognizer.view, self.view);
XCTAssertEqualObjects(self.gestureRecognizer.view, self.view);
}

- (void)testTouchesBeganSetsBeganState
Expand Down
4 changes: 2 additions & 2 deletions tests/HUBComponentImageDataBuilderTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ - (void)testPropertyAssignment

HUBComponentImageDataImplementation * const imageData = [self.builder buildWithIdentifier:identifier type:type];

XCTAssertEqual(imageData.identifier, identifier);
XCTAssertEqualObjects(imageData.identifier, identifier);
XCTAssertEqual(imageData.type, type);
XCTAssertEqualObjects(imageData.URL, self.builder.URL);
XCTAssertEqual(imageData.localImage, self.builder.localImage);
XCTAssertEqualObjects(imageData.localImage, self.builder.localImage);
XCTAssertEqualObjects(imageData.placeholderIcon.identifier, @"placeholder");
XCTAssertEqualObjects(imageData.customData, @{@"custom": @"data"});
}
Expand Down
8 changes: 4 additions & 4 deletions tests/HUBComponentModelBuilderTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ - (void)testImageConvenienceAPIs
self.builder.backgroundImage = [UIImage new];

XCTAssertEqualObjects(self.builder.mainImageDataBuilder.URL, self.builder.mainImageURL);
XCTAssertEqual(self.builder.mainImageDataBuilder.localImage, self.builder.mainImage);
XCTAssertEqualObjects(self.builder.mainImageDataBuilder.localImage, self.builder.mainImage);
XCTAssertEqualObjects(self.builder.backgroundImageDataBuilder.URL, self.builder.backgroundImageURL);
XCTAssertEqual(self.builder.backgroundImageDataBuilder.localImage, self.builder.backgroundImage);
XCTAssertEqualObjects(self.builder.backgroundImageDataBuilder.localImage, self.builder.backgroundImage);
}

- (void)testCustomImageDataBuilder
Expand Down Expand Up @@ -208,7 +208,7 @@ - (void)testChildComponentModelBuilderReuse
NSString * const childModelIdentifier = @"childModel";
id<HUBComponentModelBuilder> const childBuilder = [self.builder builderForChildWithIdentifier:childModelIdentifier];

XCTAssertEqual([self.builder builderForChildWithIdentifier:childModelIdentifier], childBuilder);
XCTAssertEqualObjects([self.builder builderForChildWithIdentifier:childModelIdentifier], childBuilder);
}

- (void)testChildTypeSameAsParent
Expand Down Expand Up @@ -307,7 +307,7 @@ - (void)testChildReferenceToParent
id<HUBComponentModel> const child = [self.builder buildForIndex:0 parent:parent];
id<HUBComponentModel> const actualParent = child.parent;

XCTAssertEqual(parent, actualParent);
XCTAssertEqualObjects(parent, actualParent);
}

- (void)testChildGrouping
Expand Down
18 changes: 9 additions & 9 deletions tests/HUBComponentModelTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ - (void)testChildComponentModelAtIndex
HUBComponentModelImplementation * const model = [self createComponentModelWithIdentifier:@"id" index:0];
model.children = childModels;

XCTAssertEqual([model childAtIndex:0], childModels[0]);
XCTAssertEqual([model childAtIndex:1], childModels[1]);
XCTAssertEqualObjects([model childAtIndex:0], childModels[0]);
XCTAssertEqualObjects([model childAtIndex:1], childModels[1]);
XCTAssertNil([model childAtIndex:2]);
}

Expand Down Expand Up @@ -154,9 +154,9 @@ - (void)testChildWithIdentifier
HUBComponentModelImplementation * const childC = [self createComponentModelWithIdentifier:@"childC" index:2];
parent.children = @[childA, childB, childC];

XCTAssertEqual([parent childWithIdentifier:@"childA"], childA);
XCTAssertEqual([parent childWithIdentifier:@"childB"], childB);
XCTAssertEqual([parent childWithIdentifier:@"childC"], childC);
XCTAssertEqualObjects([parent childWithIdentifier:@"childA"], childA);
XCTAssertEqualObjects([parent childWithIdentifier:@"childB"], childB);
XCTAssertEqualObjects([parent childWithIdentifier:@"childC"], childC);
XCTAssertNil([parent childWithIdentifier:@"noChild"]);
}

Expand All @@ -170,16 +170,16 @@ - (void)testIndexPaths
parent.children = @[childA, childB];
childB.children = @[grandchild];

XCTAssertEqual(parent.indexPath, [NSIndexPath indexPathWithIndex:0]);
XCTAssertEqualObjects(parent.indexPath, [NSIndexPath indexPathWithIndex:0]);

NSUInteger childAIndexPathArray[] = {0,0};
XCTAssertEqual(childA.indexPath, [NSIndexPath indexPathWithIndexes:childAIndexPathArray length:2]);
XCTAssertEqualObjects(childA.indexPath, [NSIndexPath indexPathWithIndexes:childAIndexPathArray length:2]);

NSUInteger childBIndexPathArray[] = {0,1};
XCTAssertEqual(childB.indexPath, [NSIndexPath indexPathWithIndexes:childBIndexPathArray length:2]);
XCTAssertEqualObjects(childB.indexPath, [NSIndexPath indexPathWithIndexes:childBIndexPathArray length:2]);

NSUInteger grandchildIndexPathArray[] = {0,1,0};
XCTAssertEqual(grandchild.indexPath, [NSIndexPath indexPathWithIndexes:grandchildIndexPathArray length:3]);
XCTAssertEqualObjects(grandchild.indexPath, [NSIndexPath indexPathWithIndexes:grandchildIndexPathArray length:3]);
}

- (void)testPropertiesThatDoNotAffectEquality
Expand Down
10 changes: 5 additions & 5 deletions tests/HUBComponentRegistryTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ - (void)testRegisteringComponentFactory

[self.registry registerComponentFactory:factory forNamespace:componentIdentifier.namespacePart];

XCTAssertEqual([self.registry createComponentForModel:componentModel], component);
XCTAssertEqualObjects([self.registry createComponentForModel:componentModel], component);
}

- (void)testRegisteringAlreadyRegisteredFactoryThrows
Expand Down Expand Up @@ -120,7 +120,7 @@ - (void)testFallbackComponentCreatedForUnknownNamespace
id<HUBComponentModel> const componentModel = [self mockedComponentModelWithComponentIdentifier:unknownNamespaceIdentifier
componentCategory:componentCategory];

XCTAssertEqual([self.registry createComponentForModel:componentModel], fallbackComponent);
XCTAssertEqualObjects([self.registry createComponentForModel:componentModel], fallbackComponent);
}

- (void)testFallbackComponentCreatedWhenFactoryReturnsNil
Expand All @@ -137,7 +137,7 @@ - (void)testFallbackComponentCreatedWhenFactoryReturnsNil
id<HUBComponentModel> const componentModel = [self mockedComponentModelWithComponentIdentifier:unknownNameIdentifier
componentCategory:componentCategory];

XCTAssertEqual([self.registry createComponentForModel:componentModel], fallbackComponent);
XCTAssertEqualObjects([self.registry createComponentForModel:componentModel], fallbackComponent);
}

- (void)testFallbackComponentsForDifferentCategories
Expand All @@ -163,8 +163,8 @@ - (void)testFallbackComponentsForDifferentCategories
id<HUBComponentModel> const componentModelB = [self mockedComponentModelWithComponentIdentifier:unknownNameIdentifier
componentCategory:componentCategoryB];

XCTAssertEqual([self.registry createComponentForModel:componentModelA], fallbackComponentA);
XCTAssertEqual([self.registry createComponentForModel:componentModelB], fallbackComponentB);
XCTAssertEqualObjects([self.registry createComponentForModel:componentModelA], fallbackComponentA);
XCTAssertEqualObjects([self.registry createComponentForModel:componentModelB], fallbackComponentB);
}

- (void)testShowcaseableComponentIdentifiers
Expand Down
4 changes: 2 additions & 2 deletions tests/HUBDefaultImageLoaderTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ - (NSURLSession *)customURLSession

- (void)imageLoader:(id<HUBImageLoader>)imageLoader didLoadImage:(UIImage *)image forURL:(NSURL *)imageURL
{
XCTAssertEqual(self.imageLoader, imageLoader);
XCTAssertEqualObjects(self.imageLoader, imageLoader);

self.loadedImage = image;
self.loadedImageURL = imageURL;
Expand All @@ -167,7 +167,7 @@ - (void)imageLoader:(id<HUBImageLoader>)imageLoader didLoadImage:(UIImage *)imag

- (void)imageLoader:(id<HUBImageLoader>)imageLoader didFailLoadingImageForURL:(NSURL *)imageURL error:(NSError *)error
{
XCTAssertEqual(self.imageLoader, imageLoader);
XCTAssertEqualObjects(self.imageLoader, imageLoader);

self.loadingError = error;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/HUBFeatureRegistryTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ - (void)testRegistrationPropertyAssignment
HUBFeatureRegistration * const registration = [self.registry featureRegistrationForViewURI:rootViewURI];
XCTAssertEqualObjects(registration.featureIdentifier, featureIdentifier);
XCTAssertEqualObjects(registration.featureTitle, @"Title");
XCTAssertEqual(registration.viewURIPredicate, viewURIPredicate);
XCTAssertEqualObjects(registration.viewURIPredicate, viewURIPredicate);
XCTAssertEqualObjects(registration.contentOperationFactories, @[contentOperationFactory]);
XCTAssertEqualObjects(registration.customJSONSchemaIdentifier, customJSONSchemaIdentifier);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/HUBIconTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ - (void)testResolvingComponentImage
imageResolver:self.imageResolver
isPlaceholder:NO];

XCTAssertEqual([icon imageWithSize:CGSizeZero color:[UIColor redColor]], image);
XCTAssertEqualObjects([icon imageWithSize:CGSizeZero color:[UIColor redColor]], image);
}

- (void)testResolvingPlaceholderImage
Expand All @@ -93,7 +93,7 @@ - (void)testResolvingPlaceholderImage
imageResolver:self.imageResolver
isPlaceholder:YES];

XCTAssertEqual([icon imageWithSize:CGSizeZero color:[UIColor redColor]], image);
XCTAssertEqualObjects([icon imageWithSize:CGSizeZero color:[UIColor redColor]], image);
}

@end
2 changes: 1 addition & 1 deletion tests/HUBInitialViewModelRegistryTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ - (void)testRegisteringRetrievingAndRemovingInitialViewModel

[self.registry registerInitialViewModel:viewModel forViewURI:viewURI];

XCTAssertEqual([self.registry initialViewModelForViewURI:viewURI], viewModel);
XCTAssertEqualObjects([self.registry initialViewModelForViewURI:viewURI], viewModel);

NSURL * const unknownViewURI = [NSURL URLWithString:@"spotify:some:other:uri"];
XCTAssertNil([self.registry initialViewModelForViewURI:unknownViewURI]);
Expand Down
4 changes: 2 additions & 2 deletions tests/HUBJSONSchemaRegistryTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ - (void)testCopyingSchema
id<HUBViewModel> const originalViewModel = [originalSchema viewModelFromJSONDictionary:dictionary];
id<HUBViewModel> const copiedViewModel = [copiedSchema viewModelFromJSONDictionary:dictionary];

XCTAssertEqual(originalViewModel.navigationItem.title, title);
XCTAssertEqual(originalViewModel.navigationItem.title, copiedViewModel.navigationItem.title);
XCTAssertEqualObjects(originalViewModel.navigationItem.title, title);
XCTAssertEqualObjects(originalViewModel.navigationItem.title, copiedViewModel.navigationItem.title);
}

- (void)testCopyingUknownSchemaReturningNil
Expand Down
4 changes: 2 additions & 2 deletions tests/HUBLiveServiceTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ - (void)testCreatingAndReusingViewController

[stream.delegate stream:stream handleEvent:NSStreamEventHasBytesAvailable];

XCTAssertEqual(self.viewController, viewController, @"View controller should have been reused");
XCTAssertEqualObjects(self.viewController, viewController, @"View controller should have been reused");

id<HUBViewModel> const newViewModel = viewController.viewModel;
XCTAssertEqualObjects(newViewModel.navigationItem.title, @"A new title!");
Expand All @@ -132,7 +132,7 @@ - (void)testCreatingAndReusingViewController

- (void)liveService:(id<HUBLiveService>)liveService didCreateViewController:(HUBViewController *)viewController
{
XCTAssertEqual(self.service, liveService);
XCTAssertEqualObjects(self.service, liveService);
self.viewController = viewController;
}

Expand Down
Loading