Implement golden-file matching for integration_test on Android and iOS devices
#160484
Conversation
This file contains hidden or 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
matanlurey
requested review from
andrewkolos,
camsim99,
johnmccutchan and
jonahwilliams
github-actions
bot
added
a: tests
…-integration-test-proxy
…-integration-test-proxy
yjbanov
approved these changes
Dec 26, 2024
| final IsolateRef testAppIsolate = await vmService.findExtensionIsolate(_kExtension); | ||
| await vmService.service.streamListen(_kEventName); | ||
| vmService.service.onEvent(_kEventName).listen((Event e) async { | ||
| if (e.extensionKind != 'compare' && e.extensionKind != 'update') { |
Contributor
There was a problem hiding this comment.
nit: !['compare', 'update'].contains(e.extensionKind) might read a little better.
| /// <String, Object?>{ | ||
| /// 'result': true /* or possibly false, in the case of 'compare' calls */ | ||
| /// } | ||
| /// ``` |
Contributor
There was a problem hiding this comment.
Are the details of the message/response public API? Not sure why these need to be under ///. Maybe make them //?
Contributor
Author
There was a problem hiding this comment.
It's not clear to me what is/isn't public API here - for example, another package (flutter_tools) needs to fulfill this API for the feature to work.
I am going to keep it as-is for now.
| if (kIsWeb) { | ||
| return false; | ||
| } | ||
| return !io.Platform.isAndroid && !io.Platform.isIOS; |
Contributor
There was a problem hiding this comment.
Are these the only platforms that separate device from host?
|
|
||
| static void _assertNotRunningOnFuchsia() { | ||
| if (!kIsWeb && io.Platform.isFuchsia) { | ||
| throw UnsupportedError('Fuchsia is not supported'); |
Contributor
There was a problem hiding this comment.
Consider a more specific error message, e.g. "integration_test golden testing does not support Fuchsia".
matanlurey
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
Contributor
|
auto label is removed for flutter/flutter/160484, due to - The status or check suite Merge Queue Guard has failed. Please fix the issues identified (or deflake) before re-applying this label. |
matanlurey
added
the
autosubmit
Contributor
|
auto label is removed for flutter/flutter/160484, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto-submit
bot
removed
the
autosubmit
matanlurey
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 6, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 7, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jan 8, 2025
…oid and iOS devices (flutter/flutter#160484)
cbracken
added a commit
to cbracken/flutter
that referenced
this pull request
Jan 9, 2025
Ignore the golden file associated with packages/integration_test/example/integration_test/integration_test_matches_golden_file.png Test (and functionality) introduced in flutter#160484.
9 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 10, 2025
Ignore the golden file associated with packages/integration_test/example/integration_test/integration_test_matches_golden_file.png Test (and functionality) introduced in #160484. Related issues: * #143299. * #160043. ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 12, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 13, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 13, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 6, 2025
…oid and iOS devices (flutter/flutter#160484)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 7, 2025
…oid and iOS devices (flutter/flutter#160484)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Work towards #143299.
Work towards #160043.
This PR implements, end-to-end, support for
matchesGoldenFilewhen (a) running withpackage:integration_test(b) on a device, such as an Android emulator, Android device, iOS simulator, or iOS device, where the runner of a test file does not have process and local-file system access.There are multiple parts to this PR; I could make it smaller than 1K lines, but the bulk of that is tests, and it would mean landing PRs that are incomplete and unused, which does not seem useful - so instead here is a quick overview of the PR's contents - questions/feedback welcome, and I am willing to break code out or land incremental refactors if requested.
flutter_platform.dart(used for iOS and Android), similar toflutter_web_platform.dart, now creates and usestest_golden_comparator.dartto proxy calls (coming from the VM service protocol) for golden-file updates and comparisons to aflutter_testerprocess. A full explanation of how (or why) it works this way is too hard to include here, but see RefactorTestGoldenComparatorto be useful for non-web (Android, iOS) integration tests #160215 for more details.VmServiceProxyGoldenFileComparator, which is a currently unused (outside of a single e2e test) comparator that forwards calls tocompareandupdateto the VM service protocol (of which, the other side of this is implemented above, influtter_platform.dart. The idea is that this comparator would be used automatically when running in an integration test on a device that requires it (similar to how web works today), but that is not wired up yet and requires additional work influtter_tools.matchesGoldenFile.