Skip to content

[camera_avfoundation] Pigeon swift migration - part 2 #10980

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.

Sign up for GitHub

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

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

[camera_avfoundation] Pigeon swift migration - part 2 #10980

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
4 changes: 4 additions & 0 deletions packages/camera/camera_avfoundation/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.9.23+3

* Migrates to Swift-based pigeon interface.

## 0.9.23+2

* Code refactor related to Swift pigeon's generated struct MediaSettings being immutable.
Expand Down
36 changes: 24 additions & 12 deletions packages/camera/camera_avfoundation/example/ios/RunnerTests/AvailableCamerasTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,14 @@ final class AvailableCamerasTest: XCTestCase {
return cameras
}

var resultValue: [FCPPlatformCameraDescription]?
cameraPlugin.availableCameras { result, error in
XCTAssertNil(error)
resultValue = result
var resultValue: [PlatformCameraDescription]?
cameraPlugin.getAvailableCameras { result in
switch result {
case .success(let result):
resultValue = result
case .failure(_):
XCTFail("Unexpected failure")
}
expectation.fulfill()
}
waitForExpectations(timeout: 30, handler: nil)
Expand Down Expand Up @@ -100,10 +104,14 @@ final class AvailableCamerasTest: XCTestCase {
return cameras
}

var resultValue: [FCPPlatformCameraDescription]?
cameraPlugin.availableCameras { result, error in
XCTAssertNil(error)
resultValue = result
var resultValue: [PlatformCameraDescription]?
cameraPlugin.getAvailableCameras { result in
switch result {
case .success(let result):
resultValue = result
case .failure(_):
XCTFail("Unexpected failure")
}
expectation.fulfill()
}
waitForExpectations(timeout: 30, handler: nil)
Expand Down Expand Up @@ -133,10 +141,14 @@ final class AvailableCamerasTest: XCTestCase {
return cameras
}

var resultValue: [FCPPlatformCameraDescription]?
cameraPlugin.availableCameras { result, error in
XCTAssertNil(error)
resultValue = result
var resultValue: [PlatformCameraDescription]?
cameraPlugin.getAvailableCameras { result in
switch result {
case .success(let result):
resultValue = result
case .failure(_):
XCTFail("Unexpected failure")
}
expectation.fulfill()
}
waitForExpectations(timeout: 30, handler: nil)
Expand Down
14 changes: 7 additions & 7 deletions ...es/camera/camera_avfoundation/example/ios/RunnerTests/CameraInitRaceConditionsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,20 @@ final class CameraInitRaceConditionsTests: XCTestCase {

// Mimic a dispose call followed by a create call, which can be triggered by slightly dragging the
// home bar, causing the app to be inactive, and immediately regain active.
cameraPlugin.disposeCamera(0) { error in
cameraPlugin.dispose(cameraId: 0) { error in
disposeExpectation.fulfill()
}

cameraPlugin.createCameraOnSessionQueue(
withName: "acamera",
settings: FCPPlatformMediaSettings.make(
with: .medium,
settings: PlatformMediaSettings(
resolutionPreset: .medium,
framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: true
)
) { result, error in
) { result in
createExpectation.fulfill()
}

Expand All @@ -69,14 +69,14 @@ final class CameraInitRaceConditionsTests: XCTestCase {

cameraPlugin.createCameraOnSessionQueue(
withName: "acamera",
settings: FCPPlatformMediaSettings.make(
with: .medium,
settings: PlatformMediaSettings(
resolutionPreset: .medium,
framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: true
)
) { result, error in
) { result in
createExpectation.fulfill()
}

Expand Down
23 changes: 14 additions & 9 deletions packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraMethodChannelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,23 @@ final class CameraMethodChannelTests: XCTestCase {
let camera = createCameraPlugin(with: avCaptureSessionMock)
let expectation = self.expectation(description: "Result finished")

var resultValue: NSNumber?
var resultValue: Int64?
camera.createCameraOnSessionQueue(
withName: "acamera",
settings: FCPPlatformMediaSettings.make(
with: FCPPlatformResolutionPreset.medium,
settings: PlatformMediaSettings(
resolutionPreset: PlatformResolutionPreset.medium,
Copy link
Contributor

@hellohuanlin hellohuanlin Feb 12, 2026

Choose a reason for hiding this comment

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

nit: can infer type

framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: true
)
) { result, error in
resultValue = result
) { result in
switch result {
case .success(let result):
resultValue = result
case .failure(_):
XCTFail("Unexpected failure")
}
expectation.fulfill()
}

Expand All @@ -60,22 +65,22 @@ final class CameraMethodChannelTests: XCTestCase {

camera.createCameraOnSessionQueue(
withName: "acamera",
settings: FCPPlatformMediaSettings.make(
with: .medium,
settings: PlatformMediaSettings(
resolutionPreset: .medium,
framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: true
)
) { result, error in
) { result in
createExpectation.fulfill()
}

waitForExpectations(timeout: 30, handler: nil)
XCTAssertNotNil(camera.camera)

let disposeExpectation = self.expectation(description: "dispose's result block must be called")
camera.disposeCamera(0) { error in
camera.dispose(cameraId: 0) { error in
disposeExpectation.fulfill()
}

Expand Down
50 changes: 18 additions & 32 deletions packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPermissionTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,17 @@ final class CameraPermissionManagerTests: XCTestCase {
let (permissionManager, mockPermissionService) = createSutAndMocks()
let expectation = self.expectation(
description: "Must complete with error if camera access was previously denied.")
let expectedError = FlutterError(
code: "CameraAccessDeniedWithoutPrompt",
message:
"User has previously denied the camera access request. Go to Settings to enable camera access.",
details: nil)

mockPermissionService.authorizationStatusStub = { mediaType in
XCTAssertEqual(mediaType, .video)
return .denied
}
permissionManager.requestCameraPermission { error in
XCTAssertEqual(error, expectedError)
XCTAssertEqual(error?.code, "CameraAccessDeniedWithoutPrompt")
XCTAssertEqual(
error?.message,
"User has previously denied the camera access request. Go to Settings to enable camera access."
)
expectation.fulfill()
}
Comment on lines 65 to 72
Copy link

@gemini-code-assist gemini-code-assist bot Feb 8, 2026

Choose a reason for hiding this comment

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

medium

There's a lot of repeated code for asserting error properties across this test file. To improve maintainability and reduce duplication, consider adding a helper function to assert the properties of a PigeonError.

For example:

private func assertPermissionError(
  _ error: PigeonError?,
  hasCode expectedCode: String,
  hasMessage expectedMessage: String,
  file: StaticString = #file,
  line: UInt = #line
) {
  XCTAssertNotNil(error, "Expected a permission error, but got nil", file: file, line: line)
  XCTAssertEqual(error?.code, expectedCode, file: file, line: line)
  XCTAssertEqual(error?.message, expectedMessage, file: file, line: line)
}

You could then simplify this test case and others like it:

permissionManager.requestCameraPermission { error in
  assertPermissionError(
    error,
    hasCode: "CameraAccessDeniedWithoutPrompt",
    hasMessage: "User has previously denied the camera access request. Go to Settings to enable camera access."
  )
  expectation.fulfill()
}

Copy link
Contributor Author

@RobertOdrowaz RobertOdrowaz Feb 10, 2026

Choose a reason for hiding this comment

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

Pls let me know if you're in favor or agains a helper in those cases. I remember that you were against in an earlier PR but it would be a much simpler helper here

Copy link
Contributor

@hellohuanlin hellohuanlin Feb 12, 2026

Choose a reason for hiding this comment

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

I'm ok with either approach. Both seem readable to me.


Expand All @@ -79,17 +78,14 @@ final class CameraPermissionManagerTests: XCTestCase {
let (permissionManager, mockPermissionService) = createSutAndMocks()
let expectation = self.expectation(
description: "Must complete with error if camera access is restricted.")
let expectedError = FlutterError(
code: "CameraAccessRestricted",
message: "Camera access is restricted.",
details: nil)

mockPermissionService.authorizationStatusStub = { mediaType in
XCTAssertEqual(mediaType, .video)
return .restricted
}
permissionManager.requestCameraPermission { error in
XCTAssertEqual(error, expectedError)
XCTAssertEqual(error?.code, "CameraAccessRestricted")
XCTAssertEqual(error?.message, "Camera access is restricted.")
expectation.fulfill()
}

Expand Down Expand Up @@ -122,10 +118,6 @@ final class CameraPermissionManagerTests: XCTestCase {
let (permissionManager, mockPermissionService) = createSutAndMocks()
let expectation = self.expectation(
description: "Must complete with error if user denied access.")
let expectedError = FlutterError(
code: "CameraAccessDenied",
message: "User denied the camera access request.",
details: nil)

mockPermissionService.authorizationStatusStub = { mediaType in
XCTAssertEqual(mediaType, .video)
Expand All @@ -137,7 +129,8 @@ final class CameraPermissionManagerTests: XCTestCase {
handler(false)
}
permissionManager.requestCameraPermission { error in
XCTAssertEqual(error, expectedError)
Copy link
Contributor

@hellohuanlin hellohuanlin Feb 12, 2026

Choose a reason for hiding this comment

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

curious why are we changing this? I don't see any problem with this approach. Is it related to equatable?

XCTAssertEqual(error?.code, "CameraAccessDenied")
XCTAssertEqual(error?.message, "User denied the camera access request.")
expectation.fulfill()
}

Expand Down Expand Up @@ -167,18 +160,17 @@ final class CameraPermissionManagerTests: XCTestCase {
let (permissionManager, mockPermissionService) = createSutAndMocks()
let expectation = self.expectation(
description: "Must complete with error if audio access was previously denied.")
let expectedError = FlutterError(
code: "AudioAccessDeniedWithoutPrompt",
message:
"User has previously denied the audio access request. Go to Settings to enable audio access.",
details: nil)

mockPermissionService.authorizationStatusStub = { mediaType in
XCTAssertEqual(mediaType, .audio)
return .denied
}
permissionManager.requestAudioPermission { error in
XCTAssertEqual(error, expectedError)
XCTAssertEqual(error?.code, "AudioAccessDeniedWithoutPrompt")
XCTAssertEqual(
error?.message,
"User has previously denied the audio access request. Go to Settings to enable audio access."
)
expectation.fulfill()
}

Expand All @@ -189,17 +181,14 @@ final class CameraPermissionManagerTests: XCTestCase {
let (permissionManager, mockPermissionService) = createSutAndMocks()
let expectation = self.expectation(
description: "Must complete with error if audio access is restricted.")
let expectedError = FlutterError(
code: "AudioAccessRestricted",
message: "Audio access is restricted.",
details: nil)

mockPermissionService.authorizationStatusStub = { mediaType in
XCTAssertEqual(mediaType, .audio)
return .restricted
}
permissionManager.requestAudioPermission { error in
XCTAssertEqual(error, expectedError)
XCTAssertEqual(error?.code, "AudioAccessRestricted")
XCTAssertEqual(error?.message, "Audio access is restricted.")
expectation.fulfill()
}

Expand Down Expand Up @@ -232,10 +221,6 @@ final class CameraPermissionManagerTests: XCTestCase {
let (permissionManager, mockPermissionService) = createSutAndMocks()
let expectation = self.expectation(
description: "Must complete with error if user denied access")
let expectedError = FlutterError(
code: "AudioAccessDenied",
message: "User denied the audio access request.",
details: nil)

mockPermissionService.authorizationStatusStub = { mediaType in
XCTAssertEqual(mediaType, .audio)
Expand All @@ -247,7 +232,8 @@ final class CameraPermissionManagerTests: XCTestCase {
handler(false)
}
permissionManager.requestAudioPermission { error in
XCTAssertEqual(error, expectedError)
XCTAssertEqual(error?.code, "AudioAccessDenied")
XCTAssertEqual(error?.message, "User denied the audio access request.")
expectation.fulfill()
}

Expand Down
30 changes: 15 additions & 15 deletions ...es/camera/camera_avfoundation/example/ios/RunnerTests/CameraPluginCreateCameraTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ final class CameraPluginCreateCameraTests: XCTestCase {
completion(nil)
}

cameraPlugin.createCamera(
withName: "camera_name",
settings: FCPPlatformMediaSettings.make(
with: .medium,
cameraPlugin.create(
cameraName: "camera_name",
settings: PlatformMediaSettings(
resolutionPreset: .medium,
framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: false)
) { result, error in
) { result in
expectation.fulfill()
}

Expand All @@ -85,15 +85,15 @@ final class CameraPluginCreateCameraTests: XCTestCase {
completion(nil)
}

cameraPlugin.createCamera(
withName: "camera_name",
settings: FCPPlatformMediaSettings.make(
with: .medium,
cameraPlugin.create(
cameraName: "camera_name",
settings: PlatformMediaSettings(
resolutionPreset: .medium,
framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: true)
) { result, error in
) { result in
expectation.fulfill()
}

Expand All @@ -117,15 +117,15 @@ final class CameraPluginCreateCameraTests: XCTestCase {
}
mockCaptureSession.canSetSessionPresetStub = { _ in true }

cameraPlugin.createCamera(
withName: "camera_name",
settings: FCPPlatformMediaSettings.make(
with: .medium,
cameraPlugin.create(
cameraName: "camera_name",
settings: PlatformMediaSettings(
resolutionPreset: .medium,
framesPerSecond: nil,
videoBitrate: nil,
audioBitrate: nil,
enableAudio: true)
) { result, error in
) { result in
expectation.fulfill()
}

Expand Down
Loading
Loading