Add a FlutterEngineRule (JUnit TestRule) and use it in FlutterRendererTest
#53361
+185
−87
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
reidbaker
approved these changes
Jun 13, 2024
|
I think we can use this rule to test backgrounding and memory trim functions. |
reidbaker
reviewed
Jun 13, 2024
| * Prepares and returns a {@link FlutterEngine} and {@link Intent} primed with an engine for tests. | ||
| */ | ||
| public final class FlutterEngineRule extends TestWatcher { | ||
| private static final Context ctx = ApplicationProvider.getApplicationContext(); |
There was a problem hiding this comment.
Error: Do not place Android context classes in static fields; this is a memory leak [StaticFieldLeak]
private static final Context ctx = ApplicationProvider.getApplicationContext();
|
Friendly ping @gmackall, I'd like to get your thoughts before merging! |
gmackall
approved these changes
Jun 13, 2024
| @@ -286,6 +284,7 @@ public void itConvertsDisplayFeatureArrayToPrimitiveArrays() { | |||
| new Rect(50, 60, 70, 80), FlutterRenderer.DisplayFeatureType.CUTOUT)); | |||
|
|
|||
| // Execute the behavior under test. | |||
| clearInvocations(fakeFlutterJNI); | |||
There was a problem hiding this comment.
What changed to make a call to clearInvocations necessary, but just in this test? (the docs say to avoid this method at all costs so I just want to know why we need it here)
There was a problem hiding this comment.
That's a good question.
The tl;dr is because calls to setViewportMetrics happen ambiently in the background with the new setup. I could change this back to creating the renderer manually (not created via engine) and avoid the need for this call. Wdut?
There was a problem hiding this comment.
I talked to Gray offline and decided it would be more work to document why this is OK than just not use it.
Resolved!
matanlurey
added
the
autosubmit
gmackall
approved these changes
Jun 13, 2024
|
auto label is removed for flutter/engine/53361, due to - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.
|
auto-submit
bot
removed
the
autosubmit
matanlurey
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 14, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 15, 2024
…`FlutterRendererTest` (flutter/engine#53361)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 15, 2024
…`FlutterRendererTest` (flutter/engine#53361)
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jun 15, 2024
…150290) flutter/engine@8167dff...9779c27 2024-06-15 [email protected] Manual roll of Dart SDK from e90b0a53e058 to dca20ab646c5 (flutter/engine#53410) 2024-06-15 [email protected] Update "Vulnerability scanning" to add the actions: read permission. (flutter/engine#53409) 2024-06-14 [email protected] Roll Skia from 136c7cc18d1e to 6f6b45e1faea (3 revisions) (flutter/engine#53408) 2024-06-14 [email protected] [Impeller] moved blur to unrotated local space, started respecting `respect_ctm` flag (flutter/engine#53377) 2024-06-14 [email protected] Roll Skia from 5560041fcfc0 to 136c7cc18d1e (4 revisions) (flutter/engine#53406) 2024-06-14 [email protected] Roll web_ui dependencies to enable the next roll of the Dart SDK (flutter/engine#53399) 2024-06-14 [email protected] Hack to prevent Safari from being backgrounded during unit tests. (flutter/engine#53402) 2024-06-14 [email protected] Manual roll Dart SDK from cabe65966815 to e90b0a53e058 (1 revision) (flutter/engine#53404) 2024-06-14 [email protected] Roll Skia from de1a50046289 to 5560041fcfc0 (1 revision) (flutter/engine#53393) 2024-06-14 [email protected] Manual roll Dart SDK from 115a9a2b26cd to cabe65966815 (1 revision) (flutter/engine#53390) 2024-06-14 [email protected] Roll Skia from 3bebb0602780 to de1a50046289 (3 revisions) (flutter/engine#53388) 2024-06-14 [email protected] Roll Skia from d248a9f99ff5 to 3bebb0602780 (1 revision) (flutter/engine#53387) 2024-06-14 [email protected] Roll Fuchsia Linux SDK from pGxbL7JoNb3yAYFw4... to BkYg1UB1jdbAZv3gA... (flutter/engine#53386) 2024-06-14 [email protected] [Impeller] restore accidentally deleted Cap/Join benchmarks (flutter/engine#53385) 2024-06-13 [email protected] Add a `FlutterEngineRule` (JUnit `TestRule`) and use it in `FlutterRendererTest` (flutter/engine#53361) 2024-06-13 [email protected] Roll Skia from b21429b0ea78 to d248a9f99ff5 (2 revisions) (flutter/engine#53383) 2024-06-13 [email protected] Roll Skia from 40bdee9eedd6 to b21429b0ea78 (3 revisions) (flutter/engine#53382) 2024-06-13 [email protected] [DisplayList] cull clip operations that can be trivially and safely ignored (flutter/engine#53367) 2024-06-13 [email protected] Roll Skia from 0f47a9333edb to 40bdee9eedd6 (2 revisions) (flutter/engine#53381) 2024-06-13 [email protected] Roll Dart SDK from aa2265e5a192 to 115a9a2b26cd (5 revisions) (flutter/engine#53380) 2024-06-13 [email protected] Roll Skia from bf5a0e0dbf41 to 0f47a9333edb (2 revisions) (flutter/engine#53378) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from pGxbL7JoNb3y to BkYg1UB1jdbA If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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.
In #53280, I'm adding lifecycle-aware methods to
SurfaceProducer.That means, in order to test that it WAI, we'll need to be running in a simulated activity, and be able to switch scenario states (i.e. to
RESUMED). This was mentioned as well in flutter/flutter#133151 as being something we want to do.This PR adds a
FlutterEngineRule, which allows the creation of a "real"FlutterEngineand anIntentthat can powerAndroidScenarioRule<FlutterActivity>. I felt bad doing all of this work for a single@Test, so I also refactored the rest of the file and cleaned things up a bit.That said, I'm happy to revert or make changes if we liked how things were setup before.