Relax the warning of unavailable tokens in gen_defaults when a default value is provided
#140872
Conversation
goderbauer
added
framework
github-actions
bot
removed
framework
github-actions
bot
added
framework
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@guidezpl I'm not entirely sure the changes to the Maybe it would be better to create the Chip theme based of the input chip spec, which I feel is the most generic. It supports both leading and trailing icons (avatar and delete) styled correctly (after #140573 is merged) For the scope of this PR maybe it would be better to keep deleteIconColor and checkmarkColor as null, which is what was being picked from the non existent token, and deal with that issue in another PR or in #140573 which is very related @TahaTesser Pinging as you are reviewing that other PR. What do you think? Here is a showcase of the chips in the Material 3 figma project.
|
github-actions
bot
removed
framework
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Is the "will affect goldens" tag supposed to be removed automatically? The changes shouldn't affect the golden tests anymore. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
TahaTesser
force-pushed
the
warn_invalid_tokens
branch
from
58002bd to
6ecf95a
Compare
TahaTesser
added
the
autosubmit
…n a default value is provided (flutter/flutter#140872)
…n a default value is provided (flutter/flutter#140872)
…n a default value is provided (flutter/flutter#140872)
…n a default value is provided (flutter/flutter#140872)
flutter/flutter@3ee8ff2...5b673c2 2024-01-23 [email protected] Roll Flutter Engine from fd0335a910b8 to b229878c57f5 (1 revision) (flutter/flutter#142051) 2024-01-23 [email protected] Remove unused clipBehavior from OverflowBar (flutter/flutter#141976) 2024-01-23 [email protected] Roll Packages from e4cbf23 to 841fe90 (7 revisions) (flutter/flutter#142047) 2024-01-23 [email protected] Roll Flutter Engine from d2855da628da to fd0335a910b8 (1 revision) (flutter/flutter#142046) 2024-01-23 [email protected] Add Share button to the SelectableRegion toolbar on Android (flutter/flutter#141447) 2024-01-23 [email protected] Relax the warning of unavailable tokens in `gen_defaults` when a default value is provided (flutter/flutter#140872) 2024-01-23 [email protected] Roll Flutter Engine from 37f68f6fc7fc to d2855da628da (1 revision) (flutter/flutter#142033) 2024-01-23 [email protected] Roll Flutter Engine from 9e582c9032e5 to 37f68f6fc7fc (1 revision) (flutter/flutter#142027) 2024-01-23 [email protected] Roll Flutter Engine from df6b15d35703 to 9e582c9032e5 (1 revision) (flutter/flutter#142026) 2024-01-23 [email protected] Roll Flutter Engine from d3dbd4225e08 to df6b15d35703 (6 revisions) (flutter/flutter#142023) 2024-01-23 [email protected] Add a comment about how to test flutter_goldens (flutter/flutter#141902) 2024-01-23 [email protected] Enable contextMenuBuilder in the absence of selectionControls (flutter/flutter#141810) 2024-01-23 [email protected] Roll Flutter Engine from b069d7f8f1fd to d3dbd4225e08 (3 revisions) (flutter/flutter#142005) 2024-01-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "hello_world app: migrate to Gradle Kotlin DSL" (flutter/flutter#142018) 2024-01-22 [email protected] Floating cursor docs (flutter/flutter#133002) 2024-01-22 [email protected] refactor: Rename filterPluginsByPlatform, cleanup Platform Strings (flutter/flutter#141780) 2024-01-22 [email protected] hello_world app: migrate to Gradle Kotlin DSL (flutter/flutter#141541) 2024-01-22 [email protected] Roll Flutter Engine from b2762f410840 to b069d7f8f1fd (4 revisions) (flutter/flutter#141993) 2024-01-22 [email protected] Remove duplicate code as suggested by natebosch. (flutter/flutter#141988) 2024-01-22 [email protected] Revert "Remove hack from PageView." (flutter/flutter#141977) 2024-01-22 [email protected] Roll Flutter Engine from d653559ae183 to b2762f410840 (3 revisions) (flutter/flutter#141978) 2024-01-22 [email protected] Do not hang on test failures of tests within `flutter_tools` (flutter/flutter#141821) 2024-01-22 [email protected] Remove unneeded expectation in test (flutter/flutter#141822) 2024-01-22 [email protected] Roll Flutter Engine from 1efe8ba6bc04 to d653559ae183 (3 revisions) (flutter/flutter#141972) 2024-01-22 [email protected] Add documentation which explains that `debugPrint` also logs in release mode (flutter/flutter#141595) 2024-01-22 [email protected] Fix `RangeSlider` throws a null-check error after `clearSemantics` is called (flutter/flutter#141965) 2024-01-22 [email protected] Roll Flutter Engine from c49989292fc1 to 1efe8ba6bc04 (1 revision) (flutter/flutter#141969) 2024-01-22 [email protected] [web] - Fix broken `TextField` in semantics mode when it's a sibling of `Navigator` (flutter/flutter#138446) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages 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 Packages: 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
| logger.printTokensUsage(verbose: true); | ||
|
|
||
| expect(printLog, contains('Tokens used: 0/0')); | ||
| expect(printLog, contains(errorColoredString('Some referenced tokens do not exist: 1'))); |
There was a problem hiding this comment.
Just a question. Why here we expect 1?
There was a problem hiding this comment.
It's somewhat explained in the comment below the test name. Maybe it could be reworded if it's not clear.
Basically, the test template reads 2 tokens, one with a default value and the other without one. In this test, there are no token values for those token names, so there is 1 warning/error for the token that didn't provide a default value.
The output would be:
Some referenced tokens do not exist: 1
color_foo_req
There was a problem hiding this comment.
Oh I see, thanks! I'm investigating this issue #142716, it seems always provide a 7 instead of 1.
There was a problem hiding this comment.
@QuncCccccc I was looking into it too, as it reminded me of a similar error I got when I was working on that PR for gen_defaults. It seems to be because of the global logger as Zachary pointed out.
I believe this PR should fix it: #142743
There was a problem hiding this comment.
Ah nice! Thanks for the super quick action:) Will take a look at the fix!
5 participants
Fixes some false positives and false negatives from the
gen_defaultstool regarding unavailable tokens in the Material spec.Fixes #140871
Before
After
These new issues that are logged are being fixed in #140573
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.