Skip to content

Conversation

@stuartmorgan-g
Copy link
Collaborator

Updates the production code to use a single instance of each of the two Pigeon host APIs, rather than a private per-file copy, so that tests can inject an alternate implementation directly instead of relying on the method channel hook and Pigeon's dartHostTestHandler.

Part of flutter/flutter#159886

Pre-Review Checklist

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

Updates the production code to use a single instance of each of the two
Pigeon host APIs, rather than a private per-file copy, so that tests can
inject an alternate implementation directly instead of relying on the
method channel hook and Pigeon's dartHostTestHandler.

Part of flutter/flutter#159886
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the in_app_purchase_storekit package to improve testability by removing the use of Pigeon's Dart test generator. The changes introduce a centralized mechanism for managing Pigeon host API instances, using global variables that can be replaced with mocks in tests. This is achieved by creating a new in_app_purchase_apis.dart file. The production code is updated to use these global API instances, and the test files are updated to use the new mocking strategy. The fake platform implementations are also updated to implement the real API interfaces, which makes the test setup more realistic. The changes are consistent, well-structured, and effectively achieve the goal of improving the testing architecture. The code quality is excellent.

@stuartmorgan-g
Copy link
Collaborator Author

This is less clean that all the other PRs for flutter/flutter#159886 because we don't have one central class to DI the alternate implementation into, and in fact there are static methods and top-level functions that use this.

The other alternative I can see here is to make a test injection method per file, like the (unused) one that used to be in sk_payment_queue_wrapper.dart‎, but that seems less maintainable.

expect(
() async => SKReceiptManager.retrieveReceiptData(),
throwsA(const TypeMatcher<PlatformException>()),
throwsA(const TypeMatcher<Exception>()),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is because the test started failing. The FakeStoreKitPlatform in this file throws an Exception, not a PlatformException. There is a different FakeStoreKitPlatform in a different file that throws a PlatformException, so it seems like this test was probably passing because somehow the wrong fake implementation was registered when it ran?

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 4, 2025
@auto-submit auto-submit bot merged commit e186c0a into flutter:main Nov 4, 2025
80 checks passed
CaoGiaHieu-dev added a commit to CaoGiaHieu-dev/packages that referenced this pull request Nov 5, 2025
* 'main' of https://github.com/CaoGiaHieu-dev/packages:
  Update packages/go_router/CHANGELOG.md
  [tool] Remove use of FETCH_HEAD (flutter#10357)
  Roll Flutter from 027f2e410241 to e5d5c01850f2 (73 revisions) (flutter#10362)
  [camera_platform_interface] Adds support for video stabilization to camera_platform_interface (flutter#10337)
  [google_maps_flutter] Raise `MapUsedAfterWidgetDisposedError` when map controller used after map disposed (flutter#9242)
  [pigeon] Replace containsKey with contains in Kotlin generator (flutter#10274)
  [video_player] Remove `package` in example `AndroidManifest.xml` file (flutter#10245)
  [flutter_svg] Fixes typo of `allowDrawingOutsideViewBox` in doc comments. (flutter#10256)
  [in_app_purchase] Remove use of Pigeon's Dart test generator (flutter#10328)
  [dependabot]: Bump com.squareup.okhttp3:okhttp from 5.1.0 to 5.3.0 in /packages/espresso/android (flutter#10348)
  Roll Flutter from 6f8abdd77820 to 027f2e410241 (26 revisions) (flutter#10335)
  [google_sign_in] Remove use of OCMock (flutter#10290)
  [interactive_media_ads] Pin iOS dependency maximum (flutter#10349)
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: in_app_purchase platform-ios platform-macos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants