[camera] Remove OCMock from tests #8342
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mchudy
force-pushed
the
feature/camera-ocmock-refactoring
branch
from
9d005b6 to
81143f5
Compare
|
Thank you for your contribution! I'm very excited about this change! It's pretty hard to review a 3000+ LOC PR. Could you break it down to smaller pieces? Smaller PR also helps with resolving potential code conflict since this plugin is being actively developed by the community. |
| @@ -1,195 +1,196 @@ | |||
| ## NEXT | |||
|
|
|||
| * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. | |||
| - Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. | |||
| - Removes OCMock from tests. | |||
There was a problem hiding this comment.
is minimum SDK bump related to OCMock removal?
There was a problem hiding this comment.
No, it's not, it's been done before in a different PR.
mchudy
force-pushed
the
feature/camera-ocmock-refactoring
branch
from
81143f5 to
13cfdcd
Compare
This was referenced Dec 27, 2024
|
@hellohuanlin I extracted two smaller PRs:
I will follow with further PRs when we get those merged as it will be tricky to keep the rest of the changes separate at this point. |
auto-submit bot
pushed a commit
that referenced
this pull request
Dec 30, 2024
Smaller part extracted from #8342 - Removes OCMock from `CameraPermissionTests` - Wraps permission methods into a new interface `FLTCameraPermissionManager` - Introduces new protocol which wraps framework methods and can be mocked directly `FLTPermissionService`
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes dependency on OCMock in unit tests and serves as a first step for full Swift migration. flutter/flutter#119109
AVFoundationclass with default implementations and mocksAVFoundationclasses with new protocolsFLTCamConfigurationfor easier usage in tests as new protocols increased the number of dependencies required to initialize a newFLTCaminstanceFLTCamto minimize the risk of introducing regressions. SplittingFLTCaminto smaller classes will happen in further PRs.AVFoundationmethods require pointers to raw objects thus I'm sometimes keeping the underlying instance exposed which is not very elegant, I'm open to any suggestions on to how we can clean this up.I took some inspiration from how it was done in case of the
in_app_purchasepackage flutter/flutter#116383Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.