[image_picker_ios] PR 1/5 Add injectable protocol seams and migrate leaf native tests to Swift Testing - #12897

Using cgImage.bytesPerRow for a 1x1 destination context is dangerous because cgImage.bytesPerRow is the row width of the original image (which is much larger than 4 bytes). Since the destination buffer argb is only a single UInt32 (4 bytes), specifying a large bytesPerRow can lead to memory corruption, undefined behavior, or AddressSanitizer (ASan) failures.

Additionally, using cgImage.bitsPerComponent and cgImage.bitmapInfo directly might fail if the source image is in a different format (e.g., grayscale or 16-bit).

We should initialize a standard 32-bit ARGB 1x1 context instead:

Good catch. The 1×1 context now uses MemoryLayout.size(ofValue: argb) so it doesn’t write past the UInt32. Left bitsPerComponent and bitmapInfo as the source image’s so the color assertions stay the same

Using a hardcoded filename like "image_picker_test.jpg" in the shared temporary directory can lead to test flakiness or conflicts when tests are run in parallel. It is safer to use a unique filename using UUID().uuidString.

Done! Unique UUID name, and the file is removed with defer

The temporary files created by saveImage are not cleaned up at the end of this test. We should use defer to ensure they are removed from the disk.

Done for each saveImage path

The temporary file created for the PNG image is not cleaned up. We should use defer to ensure it is removed.

Done for each saveImage path

The temporary file created by saveImage is not cleaned up. We should use defer to ensure it is removed.

    let savedPathJPG = FLTImagePickerPhotoAssetUtil.saveImage(
      withPickerInfo: nil, image: imageJPG, imageQuality: nil)
    defer { try? FileManager.default.removeItem(atPath: savedPathJPG) }

Done for each saveImage path

The temporary file created by saveImage is not cleaned up. We should use defer to ensure it is removed.

Done for each saveImage path

The temporary file created by saveImage is not cleaned up. We should use defer to ensure it is removed.

Done for each saveImage path

The temporary files created during verifySavingImage are not cleaned up. We should use defer to ensure they are removed from the disk.

    let path = try #require(savedPath)
    defer { try? FileManager.default.removeItem(atPath: path) }
    #expect(FileManager.default.fileExists(atPath: path))
    #expect(URL(fileURLWithPath: path).pathExtension == expectedExtension)

Done in verifySavingImage, the GIF test, and the orientation test

The temporary file created for the GIF image is not cleaned up. We should use defer to ensure it is removed.

Done in verifySavingImage, the GIF test, and the orientation test

The temporary file created for the oriented image is not cleaned up. We should use defer to ensure it is removed.

Done in verifySavingImage, the GIF test, and the orientation test

Not sure I understand the part about failing xcodebuild? You mean they are going to fail at compile time when you run xcodebuild to build the tests or at test time when you run xcodebuild test?

Test time, not compile time. #expect inside savedPathBlock is not recorded on the test, so xcodebuild test can exit 0 even when that expectation fails. The comment now says that. Assertions stay after the operation finishes

Nonblocking, but is it possible to do this to avoid using nonisolated(unsafe)?

let result: Result<String, FlutterError> = try #require(
  await withCheckedContinuation { continuation in
    let operation = FLTPHPickerSaveImageToPathOperation(
      result: pickerResult,
      maxHeight: maxHeight,
      maxWidth: maxWidth,
      desiredImageQuality: desiredImageQuality,
      fullMetadata: fullMetadata,
      savedPathBlock: { path, error in
        if let path {
          continuation.resume(returning: .success(path))
        } else {
          continuation.resume(returning: .failure(error!))
        }
      }
    )
    if let operation {
      operation.start()
    } else {
      continuation.resume(returning: nil)
    }
  }
)

Done! savedPathBlock resumes the continuation with (path, error), so the helper no longer stores those values. I kept both optionals instead of Result: both can be nil, and error! would crash there. A nil operation resumes with nil, and #require fails the test. The block runs before setFinished:, so this also drops the completionBlock wait

The comments may need updating since confirmation is no longer used.

Updated. The comment no longer mentions confirmation