Skip to content

Conversation

@matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Dec 13, 2024

Part of #160043, makes it easier to add #160131.

This PR has no functional changes to any of the code, but does refactor both the code and tests:

  • Makes a number of always non-null but not migrated to non-null properties, well, not-null
  • Creates two concrete methods (update and compare versus a positional nullable boolean)
  • Uses type signatures instead of String? to explain the possible results of the methods
  • Renames the mysterious shellPath variable to flutterTesterBinPath
  • Expands and rewrites internally-facing doc comments
  • Moves WebRenderer environment variable setting to flutter_web_platform.dart
  • Makes the tests have less duplication, and check for update/compare cases

After this PR, I can use it in the non-web branch of the Flutter tool without any hacks or TODOS :)

/cc @eyebrowsoffire (trivial web refactoring), @camsim99 (changes being made to tool).

@matanlurey matanlurey requested review from andrewkolos, eyebrowsoffire and jonahwilliams and removed request for eyebrowsoffire December 13, 2024 01:54
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Dec 13, 2024
@matanlurey
Copy link
Contributor Author

Ping @andrewkolos @jonahwilliams :)

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 16, 2024
@matanlurey
Copy link
Contributor Author

@andrewkolos Your feedback is still welcome, and I'm happy to make adjustments in a follow-up PR.

@auto-submit auto-submit bot added this pull request to the merge queue Dec 16, 2024
Merged via the queue into flutter:master with commit 18f56e3 Dec 16, 2024
146 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 17, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 28, 2024
…iOS devices (#160484)

Work towards #143299.
Work towards #160043.

---

This PR implements, end-to-end, support for `matchesGoldenFile` when (a)
running with `package: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.

1. Augmented `flutter_platform.dart` (used for iOS and Android), similar
to
[`flutter_web_platform.dart`](https://github.com/flutter/flutter/blob/1398dc7eecb696d302e4edb19ad79901e615ed56/packages/flutter_tools/lib/src/test/flutter_web_platform.dart#L117-L128),
now creates and uses
[`test_golden_comparator.dart`](https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/test/test_golden_comparator.dart)
to proxy calls (coming from the VM service protocol) for golden-file
updates and comparisons to a `flutter_tester` process. A full
explanation of how (or why) it works this way is too hard to include
here, but see #160215 for more
details.
1. Added `VmServiceProxyGoldenFileComparator`, which is a currently
unused (outside of a single e2e test) comparator that forwards calls to
`compare` and `update` to the VM service protocol (of which, the other
side of this is implemented above, in `flutter_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 in
`flutter_tools`.
1. Added two unit tests (of both the client and server), and a full
e2e-test using it to run `matchesGoldenFile`.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 8, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants