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

Add ability to exclude OS version from snapshot file name #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add ability to exclude OS version from snapshot file name #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions FBSnapshotTestCase/FBSnapshotTestCase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,21 @@
/**
When @c YES appends the name of the device model and OS to the snapshot file name.
The default value is @c NO.

@see includeOSVersionInFilename
*/
@property (readwrite, nonatomic, assign, getter=isDeviceAgnostic) BOOL deviceAgnostic;

/**
Used in conjunction with @c deviceAgnostic, when @c YES the OS version will be included in the snapshot file name.
The default value is @c YES.

@attention @c deviceAgnostic needs to be set to @c YES for this to take effect

@see deviceAgnostic
*/
@property (readwrite, nonatomic, assign) BOOL includeOSVersionInFilename;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this from a BOOL to an enum where we could select what information to append? The default value will accommodate all of them. I want to avoid a specific property just for the name OS version.

typedef NS_OPTIONS(NSInteger, FBSnapshotTestDeviceAgnosticOption) {
  FBSnapshotTestDeviceAgnosticOptionModel         = 1 << 0,
  FBSnapshotTestDeviceAgnosticOptionSystemVersion = 1 << 1,
  FBSnapshotTestDeviceAgnosticOptionScreenSize    = 1 << 2,
};

@property (readwrite, nonatomic, assign) FBSnapshotTestDeviceAgnosticOption deviceAgnosticOptions;

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that is a nicer solution. I was planning on a follow up PR to this one that used more descriptive device names in the snapshot filenames, rather than just using model names, so maybe you can give me some feedback on that idea now, and I might be able to incorporate it into this?

Basically my rationale is that it isn't easy to determine if a snapshot is for an iPhone 6, or a 6 Plus from the filename. While the use of the device model (iPhone or iPad) and size dimensions give you some idea of what it is, having iPhone6 or iPhone6Plus in the file name is more readable.

Would it be preferable to replace device model with more descriptive device names, or make descriptive device names a further option? I'd prefer to replace, but it would be a breaking change for everybody using deviceAgnostic = true.


/**
When YES, renders a snapshot of the complete view hierarchy as visible onscreen.
There are several things that do not work if renderInContext: is used.
Expand Down
11 changes: 11 additions & 0 deletions FBSnapshotTestCase/FBSnapshotTestCase.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ - (void)setDeviceAgnostic:(BOOL)deviceAgnostic
_snapshotController.deviceAgnostic = deviceAgnostic;
}

- (BOOL)includeOSVersionInFilename
{
return _snapshotController.includeOSVersionInFilename;
}

- (void)setIncludeOSVersionInFilename:(BOOL)include
{
NSAssert1(_snapshotController, @"%s cannot be called before [super setUp]", __FUNCTION__);
_snapshotController.includeOSVersionInFilename = include;
}

- (BOOL)usesDrawViewHierarchyInRect
{
return _snapshotController.usesDrawViewHierarchyInRect;
Expand Down
2 changes: 1 addition & 1 deletion FBSnapshotTestCase/FBSnapshotTestCasePlatform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ NSOrderedSet *FBSnapshotTestCaseDefaultSuffixes(void);

@returns An @c NSString object containing the passed @c fileName with the device model, OS and screen size appended at the end.
*/
NSString *FBDeviceAgnosticNormalizedFileName(NSString *fileName);
NSString *FBDeviceAgnosticNormalizedFileName(NSString *fileName, BOOL includeOSVersion);

#ifdef __cplusplus
}
Expand Down
25 changes: 15 additions & 10 deletions FBSnapshotTestCase/FBSnapshotTestCasePlatform.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,25 @@ BOOL FBSnapshotTestCaseIs64Bit(void)
return [suffixesSet copy];
}

NSString *FBDeviceAgnosticNormalizedFileName(NSString *fileName)
NSString *FBDeviceAgnosticNormalizedFileName(NSString *fileName, BOOL includeOSVersion)
{
NSMutableString *deviceAgnosticFilename = [fileName mutableCopy];

UIDevice *device = [UIDevice currentDevice];
[deviceAgnosticFilename appendFormat:@"_%@", device.model];

if (includeOSVersion) {
[deviceAgnosticFilename appendFormat:@"%@", device.systemVersion];
}

UIWindow *keyWindow = [[UIApplication sharedApplication] fb_strictKeyWindow];
CGSize screenSize = keyWindow.bounds.size;
NSString *os = device.systemVersion;

fileName = [NSString stringWithFormat:@"%@_%@%@_%.0fx%.0f", fileName, device.model, os, screenSize.width, screenSize.height];

[deviceAgnosticFilename appendFormat:@"_%.0fx%.0f", screenSize.width, screenSize.height];

NSMutableCharacterSet *invalidCharacters = [NSMutableCharacterSet new];
[invalidCharacters formUnionWithCharacterSet:[NSCharacterSet whitespaceCharacterSet]];
[invalidCharacters formUnionWithCharacterSet:[NSCharacterSet punctuationCharacterSet]];
NSArray *validComponents = [fileName componentsSeparatedByCharactersInSet:invalidCharacters];
fileName = [validComponents componentsJoinedByString:@"_"];

return fileName;
}
NSArray *validComponents = [deviceAgnosticFilename componentsSeparatedByCharactersInSet:invalidCharacters];

return [validComponents componentsJoinedByString:@"_"];
}
12 changes: 12 additions & 0 deletions FBSnapshotTestCase/FBSnapshotTestController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,21 @@ extern NSString *const FBDiffedImageKey;
/**
When @c YES appends the name of the device model and OS to the snapshot file name.
The default value is @c NO.

@see includeOSVersionInFilename
*/
@property (readwrite, nonatomic, assign, getter=isDeviceAgnostic) BOOL deviceAgnostic;

/**
Used in conjunction with @c deviceAgnostic, when @c YES the OS version will be included in the snapshot file name.
The default value is @c YES.

@attention @c deviceAgnostic needs to be set to @c YES for this to take effect

@see deviceAgnostic
*/
@property (readwrite, nonatomic, assign) BOOL includeOSVersionInFilename;

/**
Uses drawViewHierarchyInRect:afterScreenUpdates: to draw the image instead of renderInContext:
*/
Expand Down
3 changes: 2 additions & 1 deletion FBSnapshotTestCase/FBSnapshotTestController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ - (instancetype)initWithTestName:(NSString *)testName
if (self = [super init]) {
_testName = [testName copy];
_deviceAgnostic = NO;
_includeOSVersionInFilename = YES;

_fileManager = [[NSFileManager alloc] init];
}
Expand Down Expand Up @@ -234,7 +235,7 @@ - (NSString *)_fileNameForSelector:(SEL)selector
}

if (self.isDeviceAgnostic) {
fileName = FBDeviceAgnosticNormalizedFileName(fileName);
fileName = FBDeviceAgnosticNormalizedFileName(fileName, self.includeOSVersionInFilename);
}

if ([[UIScreen mainScreen] scale] > 1) {
Expand Down
17 changes: 16 additions & 1 deletion FBSnapshotTestCaseTests/FBSnapshotControllerTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,25 @@ - (void)testFailedImageWithDeviceAgnosticShouldHaveModelOnName
SEL selector = @selector(isDeviceAgnostic);
[controller referenceImageForSelector:selector identifier:@"" error:&error];
XCTAssertNotNil(error);
NSString *deviceAgnosticReferencePath = FBDeviceAgnosticNormalizedFileName(NSStringFromSelector(selector));
NSString *deviceAgnosticReferencePath = FBDeviceAgnosticNormalizedFileName(NSStringFromSelector(selector), true);
XCTAssertTrue([(NSString *)[error.userInfo objectForKey:FBReferenceImageFilePathKey] containsString:deviceAgnosticReferencePath]);
}

- (void)testFilenameDoesNotContainOSVersionWhenIncludeOSInFilenameIsFalse
Copy link
Contributor

Choose a reason for hiding this comment

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

and then we right a robust set of tests

{
FBSnapshotTestController *controller = [[FBSnapshotTestController alloc] initWithTestClass:nil];
[controller setDeviceAgnostic:YES];
[controller setIncludeOSVersionInFilename:NO];

[controller setReferenceImagesDirectory:@"/dev/null/"];
NSError *error = nil;
SEL selector = @selector(includeOSVersionInFilename);
[controller referenceImageForSelector:selector identifier:@"" error:&error];
XCTAssertNotNil(error);
NSString *includeOSVersionInFilenameReferencePath = FBDeviceAgnosticNormalizedFileName(NSStringFromSelector(selector), false);
XCTAssertTrue([(NSString *)[error.userInfo objectForKey:FBReferenceImageFilePathKey] containsString:includeOSVersionInFilenameReferencePath]);
}

#pragma mark - Private helper methods

- (UIImage *)_bundledImageNamed:(NSString *)name type:(NSString *)type
Expand Down