ios-snapshot-test-case does not work properly if UIApperance attributes change intrinsic content size of view being tested

[jgongo]

I was having a problem regarding a view that wasn't being properly recorded when I changed an attribute of the view using the UIAppearance proxy, if that attribute would change the intrinsic content size of the view.

The culprit seems to be in the following method in UIImage+Snapshot.m:

+ (UIImage *)fb_imageForView:(UIView *)view
{
  CGRect bounds = view.bounds;
  NSAssert1(CGRectGetWidth(bounds), @"Zero width for view %@", view);
  NSAssert1(CGRectGetHeight(bounds), @"Zero height for view %@", view);

  // If the input view is already a UIWindow, then just use that. Otherwise wrap in a window.
  UIWindow *window = [view isKindOfClass:[UIWindow class]] ? (UIWindow *)view : view.window;
  BOOL removeFromSuperview = NO;
  if (!window) {
    window = [[UIApplication sharedApplication] fb_strictKeyWindow];
  }

  if (!view.window && view != window) {
    [window addSubview:view];
    removeFromSuperview = YES;
  }

  UIGraphicsBeginImageContextWithOptions(bounds.size, NO, 0);
  [view layoutIfNeeded];
  [view drawViewHierarchyInRect:view.bounds afterScreenUpdates:YES];

  UIImage *snapshot = UIGraphicsGetImageFromCurrentImageContext();
  UIGraphicsEndImageContext();

  if (removeFromSuperview) {
    [view removeFromSuperview];
  }

  return snapshot;
}

The problem is the following:

1. In the beginning of the method the current bounds of the view to be tested are stored in the bounds variable.
2. The view is added to a window (if it still didn't belong to one). This fires the appearance mechanism, that may set a property that calls setNeedsLayout
3. Then an image context is created with the size of the original bounds.
4. Then layoutIfNeeded is invoked on the view being tested. This may change the original dimensions of the view if the intrinsic content size changes
5. The view is drawn using its current bounds, but the image context is still using the original bounds

As a result, if the view has a greater intrinsic content size due to some change in appearance properties (for example, a greater font) it is clipped as it's being drawn in a smaller image context.

Is it possible to invoke layoutIfNedeedbefore UIGraphicsBeginImageContextWithOptions to use the updated size of the view? Or am I missing anything? BTW, the view I'm testing seems to be behaving correctly when resized after changing the font to a greater one in an application, so I think it's correctly implemented (I may share it if you want)

[jgongo]

I have just noticed that this also happens with any property that changes the intrinsic content size. For example, setting a large text that makes the view grow.

[Grubas7]

@jgongo could you provide sample app?

[jgongo]

Here you have it. I'm using Nimble-Snapshots as a wrapper over ios-snapshot-test-case, but I've checked that the parameters are being correctly passed down. If they weren't, appearance properties wouldn't be applied.

Here you have the relevant piece of code:

UIButton.appearance().contentEdgeInsets = UIEdgeInsets(top: 15, left: 15, bottom: 15, right: 15)
                
let button = UIButton(type: .system)
button.setTitle("Click me!", for: .normal)
button.layer.borderColor = UIColor.red.cgColor
button.layer.borderWidth = 2
                
button.snp.makeConstraints({ (make) in
    make.width.greaterThanOrEqualTo(65)
})
button.sizeToFit()
                
expect(button).to(recordSnapshot(usesDrawRect: true))

I've added a border to the button so you can check its bounds and a width constraint so it gets resized when auto layout does its magic. If you comment the first line (setting the content edge insets through the appearance proxy) you'll see the button snapshot gets properly recorded. If you leave it you'll see the button gets clipped on the bottom and right.

SnapshotTestCase.zip

[jgongo]

I have just created a PR to solve the issue: #228

[jgongo]

Hi... is this project alive? I opened this issue more than 4 months ago, I provided a PR solving the issue, and yet no progress 😞

[nscoding]

Hey, I understand and I am really sorry for this. Unfortunately I recently archived the project as we no longer use this internally and deciding about API improvements without being a consumer is incredibly difficult. I am working on a solution though to provide a better owner for it. I'll keep you posted.

[jgongo]

I'm curious... you no longer do view based testing? Or do you use anything else?

[nscoding]

Our number of tests grew (>10k) and intentional changes were slowing us down so we built another system (similar to this one) that doesn't rely on XCTestCase. It simply records images and stores them in the DB. This runs on CI job and If there is a change, we file a task and bisect to the offending commit.

[remlostime]

@nscoding Is it possible to share the timeline for new FBSnapshotTest framework? And will FB opensource it?

[freak4pc]

@nscoding I imagine you won't Open source this new piece of software, but any chance you can give a quick rundown (or just notes) on what was the selected solution? How do you make snapshots of your Views/Layers without a dependency on XCTestCase? Sounds very interesting and I'd love the opportunity to dig deeper into it.