- Notifications
You must be signed in to change notification settings - Fork30k
Comments
fix(web_ui): use static whitelist for image codec tests#182648
fix(web_ui): use static whitelist for image codec tests#182648harryterkelsen wants to merge 3 commits intoflutter:masterfrom
Conversation
The `codecs_test.dart` previously fetched all images from `/test_images/`dynamically. This caused flakiness when Skia added new images that werepurposely undecodable or problematic for the Flutter web engine.This change introduces a static whitelist of 187 verified test imagesand removes the dynamic fetching logic. It also cleans up obsoleteskip conditions for images that are no longer in the whitelist.Verified by running the tests on Chrome via `felt`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Code Review
The pull request correctly transitions the image codec tests to use a static whitelist instead of dynamic discovery, which should significantly reduce flakiness caused by unexpected changes in the test image directory. The changes include the addition of a comprehensive_kTestImages list, removal of unused imports, and cleanup of obsolete logic. One minor oversight was found where a skip condition for an image not present in the new whitelist was retained.
Uh oh!
There was an error while loading.Please reload this page.
mdebbar left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Uh oh!
There was an error while loading.Please reload this page.
auto label is removed for flutter/flutter/182648, Failed to enqueue flutter/flutter/182648 with HTTP 400: GraphQL mutate failed. |
autosubmit label was removed for flutter/flutter/182648, because - The status or check suiteWindows tool_integration_tests_4_9 has failed. Please fix the issues identified (or deflake) before re-applying this label. |
The
codecs_test.dartpreviously fetched all images from/test_images/dynamically. This caused flakiness when Skia added new images that were purposely undecodable or problematic for the Flutter web engine.This change introduces a static whitelist of verified test images and removes the dynamic fetching logic. It also cleans up obsolete skip conditions for images that are no longer in the whitelist.
Fixes#182629
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel onDiscord.
Note: The Flutter team is currently trialing the use ofGemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.