Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jonahwilliams
Copy link
Contributor

Work towards flutter/flutter#103724

TLDR: We want to make the tester able to load assets by passing flutter-asset-dir. Unfortunately that also causes tests to fallback to bundled fonts ahead of Ahem font which is massively breaking to scuba and not necessarily something we want to be easy. Add a flag that prevents registering fonts located in the asset bundle.

@flutter-dashboard

This comment was marked as outdated.

@jonahwilliams
Copy link
Contributor Author

I've added tests that assert that with the flag passed a bundled roboto font is not used (verified this test fails with the flag removed). I also added a test that we can still add new fonts by using the loadFont API

DEF_SWITCH(DisableAssetFonts,
"disable-asset-fonts",
"Prevents usage of any non-test fonts unless they were explicitly "
"Loaded via dart:ui font APIs. This option is only available on the "
Copy link
Member

Choose a reason for hiding this comment

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

How is it kept only available in the test shell?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh right. hmm. I think should probably ifdef this out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, I guess the thinking is that these switches aren't on an allowlist or surfaced through the embedder, but this one and use-test-fonts are technically still there. We should fix that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh, but if we change a define for switches then we'd have to compile this twice for each host platform

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a debug mode guard

@jonahwilliams jonahwilliams requested a review from zanderso May 13, 2022 20:01
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm

@jonahwilliams jonahwilliams added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 13, 2022
@fluttergithubbot fluttergithubbot merged commit 1f06f24 into flutter:main May 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 14, 2022
@jonahwilliams jonahwilliams deleted the tester_bundle_stuff branch May 14, 2022 02:07
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants