Skip to content

[camera_avfoundation] Migrate tests to Swift - part 2 #8613

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

Merged

Conversation

FirentisTFW
Copy link
Contributor

@FirentisTFW FirentisTFW commented Feb 12, 2025

This PR migrates some of Objective-C tests to Swift as a part of Swift migration: flutter/flutter/issues/119109.

I kept the names of test cases the same.

Tests migrated in this PR:

  • CameraCaptureSessionQueueRaceConditionTests
  • CameraMethodChannelTests
  • CameraOrientationTests
  • CameraSettingsTests
  • CameraSessionPresetsTests

Pre-launch Checklist

Comment on lines 90 to 96
// AVAssetWriterInput needs these three keys, otherwise it throws.
var outputSettingsWithRequiredKeys = outputSettings ?? [:]
outputSettingsWithRequiredKeys[AVVideoCodecKey] = AVVideoCodecType.h264
outputSettingsWithRequiredKeys[AVVideoWidthKey] = 1280
outputSettingsWithRequiredKeys[AVVideoHeightKey] = 720

return AVAssetWriterInput(mediaType: .video, outputSettings: outputSettingsWithRequiredKeys)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Objective-C it worked without these three keys, although the documentation still says that they're needed. For Swift, I had to add them here, otherwise AVAssetWriterInput constructor throws, as per the documentation:

When using this initializer, a video settings dictionary must be fully specified, meaning that it must contain AVVideoCodecKey, AVVideoWidthKey, and AVVideoHeightKey.

and

This method throws an exception for any of the following reasons:
  ...
  - for video inputs, the output settings do not contain a required key (AVVideoCodecKey, AVVideoWidthKey, AVVideoHeightKey)

And in this test we only have AVVideoCompressionPropertiesKey in options here.

@FirentisTFW FirentisTFW force-pushed the feature/camera-swift-test-migration-part2 branch from 3227f58 to 612c425 Compare February 13, 2025 15:26
@FirentisTFW FirentisTFW marked this pull request as ready for review February 13, 2025 15:43
Copy link
Contributor

@hellohuanlin hellohuanlin left a comment

Choose a reason for hiding this comment

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

some nits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was migrated to Swift in the previous PR, but I did not remove the ObjectiveC version - now it's done.

@FirentisTFW
Copy link
Contributor Author

@stuartmorgan Would you like to do a secondary review or can I land it?

@stuartmorgan-g
Copy link
Contributor

I only skimmed it, since it doesn't need a second full review, but other than the missing test file this looks good to land.

@FirentisTFW FirentisTFW added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 18, 2025
@auto-submit auto-submit bot merged commit 89f6e93 into flutter:main Feb 18, 2025
82 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 18, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Feb 18, 2025
flutter/packages@8542af3...cb4fb13

2025-02-18 [email protected]
[shared_preferences] Fix JSON parsing issue with _decodeValue
(flutter/packages#8211)
2025-02-18 [email protected] [camera_avfoundation] Migrate
tests to Swift - part 2 (flutter/packages#8613)
2025-02-18 [email protected] [google_sign_in] Adopt task queues
for Android (flutter/packages#8622)
2025-02-17 [email protected] Roll Flutter from
892f9c1 to e8f34a9 (71 revisions) (flutter/packages#8614)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-ios platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants