Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
knellr opened this issue Feb 16, 2023 · 6 comments

Comments

@knellr
Copy link

knellr commented Feb 16, 2023

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

  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

  • swift-snapshot-testing version 1.11.0
  • Xcode 14.2
  • Swift 5.7
  • OS: iOS 16

Additional context

xctestresult for Incorrectly passing run (Bitrise):

Test-SnapshotIssue.xcresult.zip

Logs

xcodebuild_test.log

@nshelleyacorns
Copy link

nshelleyacorns commented Feb 16, 2023

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 #702. (But it still hasn't been merged and no word from the maintainers... 😞)

@knellr
Copy link
Author

knellr commented Feb 16, 2023

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
Copy link
Author

knellr commented Feb 16, 2023

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
Copy link
Author

knellr commented Feb 17, 2023

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

@tdrhq
Copy link

tdrhq commented May 4, 2023

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
Copy link
Author

knellr commented Nov 3, 2023

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

@knellr knellr closed this as completed Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants