pointfreeco / swift-snapshot-testing

📸 Delightful Swift snapshot testing.
https://www.pointfree.co/episodes/ep41-a-tour-of-snapshot-testing
MIT License
3.78k stars 580 forks source link

Perceptual precision causing differences to be missed in Bitrise CI due to failing CIContext.render() calls #710

Closed knellr closed 12 months ago

knellr commented 1 year ago

Describe the bug

When running with perceptual precision < 1 we are observing that comparisons are picking up failures correctly locally, but are passing incorrectly on Bitrise CI.

To Reproduce

Run a snapshot test in Bitrise that is expected to fail with a perceptualPrecision < 1, for example as shown in the following project:

SnapshotIssue.zip

The issue appears to be with the following code (and similar) not updating the averagePixel var. Because it's initialised as zero, the tests pass - see https://github.com/pointfreeco/swift-snapshot-testing/blob/e408bf983609cfa8b4656a364086ba4ed91f742c/Sources/SnapshotTesting/Snapshotting/UIImage.swift#L188

  var averagePixel: Float = 0
  let context = CIContext(options: [.workingColorSpace: NSNull(), .outputColorSpace: NSNull()])
  context.render(
    thresholdOutputImage.applyingFilter("CIAreaAverage", parameters: [kCIInputExtentKey: new.extent]),
    toBitmap: &averagePixel,
    rowBytes: MemoryLayout<Float>.size,
    bounds: CGRect(x: 0, y: 0, width: 1, height: 1),
    format: .Rf,
    colorSpace: nil
  )

I'm not familiar with this call and this may be a Bitrise specific issue as they're running in VMs (I have raised a ticket with Bitrise) but I think it should be possible to make the tests fail if this scenario is hit.

Expected behavior

Test should fail

Environment

Additional context

xctestresult for Incorrectly passing run (Bitrise):

Test-SnapshotIssue.xcresult.zip

Logs

xcodebuild_test.log

nshelleyacorns commented 1 year ago

I'm pretty sure this is because perceptual precision requires metal and VMs don't support metal. I ran into the same issue on CircleCI. Unfortunately, without metal support all tests using perceptual precision would pass, but that has been fixed in https://github.com/pointfreeco/swift-snapshot-testing/pull/702. (But it still hasn't been merged and no word from the maintainers... 😞)

knellr commented 1 year ago

Ah yes, it seems likely the same thing. Interestingly Bitrise did have Metal support, but apparently it was turned off last March and I guess maybe not turned back on again https://bitrise.io/blog/post/run-faster-ios-ui-tests-via-metal-support

knellr commented 1 year ago

Ran the branch in #702 and got the failure I needed all along

Assertions: Assertion Failure at SnapshotIssueTests.swift:18: failed - Snapshot does not match reference.

@−
"file:///Users/vagrant/git/SnapshotIssueTests/__Snapshots__/SnapshotIssueTests/testSnapshot.1.png"
@+
"file:///Users/vagrant/Library/Developer/XCTestDevices/39F9E0E8-C99C-4BD6-91B7-14021B8BBF34/data/Containers/Data/Application/1F6B607B-DD35-4997-8408-9496F3F51C8F/tmp/SnapshotIssueTests/testSnapshot.1.png"

To configure output for a custom diff tool, like Kaleidoscope:

    SnapshotTesting.diffTool = "ksdiff"

Failed to compare snapshots. Metal is required for perceptuallyCompare, but not available on this machine.
  File: SnapshotIssueTests.swift:18
knellr commented 1 year ago

Bitrise have confirmed that they currently only support Metal in their M1 environments.

tdrhq commented 1 year ago

I think the issue here is recording screenshots in a different environment than where it's being verified from. Ideally you'd record screenshots in CI.

knellr commented 12 months ago

Closing this as this is no longer an issue in Bitrise, though #666 or #702 may help in equivalent environments