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

Avoiding using separate FlutterLoaders in scenario_app #50927

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Feb 23, 2024

Fixes flutter/flutter#144046

When we use the same FlutterLoader from the injector, the initialization method knows it doesn't have to do as much work - using separate Flutter loaders (in this case for every test that derives from TestActivity, which is basically all of them) causes spurious warning messages to be printed for every test.

Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

Thank you!

What do you think of making these warnings a failure case for the test going forward?

@dnfield
Copy link
Contributor Author

dnfield commented Feb 23, 2024

Thank you!

What do you think of making these warnings a failure case for the test going forward?

I think that's fine, but I could also imagine a case where some test gets written to explicitly test that actually calling it twice with different instances doesn't crash. But if we go there we should probably just make the method better at detecting re-entrant calls from multiple instances instead of just one instance.

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2024
@auto-submit auto-submit bot merged commit 65a6d6e into main Feb 23, 2024
25 checks passed
@auto-submit auto-submit bot deleted the dnfield-patch-1 branch February 23, 2024 23:46
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 24, 2024
…144073)

flutter/engine@7380422...3c036c0

2024-02-24 737941+loic-sharma@users.noreply.github.com [Windows] Refactor the a11y announcement test (flutter/engine#50888)
2024-02-23 dnfield@google.com Avoiding using separate FlutterLoaders in scenario_app (flutter/engine#50927)

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 jimgraham@google.com,rmistry@google.com,zra@google.com 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
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
3 participants