Skip to content
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

[camera] Remove OCMock from tests #8342

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

mchudy
Copy link

@mchudy mchudy commented Dec 23, 2024

This PR removes dependency on OCMock in unit tests and serves as a first step for full Swift migration. flutter/flutter#119109

  • Introduces wrapper protocols around AVFoundation class with default implementations and mocks
  • Replaces usages of AVFoundation classes with new protocols
  • Refactors dependency injection with FLTCamConfiguration for easier usage in tests as new protocols increased the number of dependencies required to initialize a new FLTCam instance
  • I tried to minimize changes to FLTCam to minimize the risk of introducing regressions. Splitting FLTCam into smaller classes will happen in further PRs.
  • Relying on the new protocols in some cases is cumbersome because AVFoundation methods 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_purchase package flutter/flutter#116383

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@mchudy mchudy force-pushed the feature/camera-ocmock-refactoring branch from 9d005b6 to 81143f5 Compare December 23, 2024 11:03
@hellohuanlin
Copy link
Contributor

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

is minimum SDK bump related to OCMock removal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants