Repository navigation
[tool][material_ui] Only run test files containing the package's tag with dart-test --package-tags (currently just material_ui web tests) - #13536
Conversation
ae51561 to
b0e2b33
Compare
Reverts flutter#13537, which temporarily excluded material_ui from the non-Wasm web unit tests because the shard containing it was exceeding its 90 minute timeout. With dart-test --package-tags now only loading the test files that contain the package's tag, material_ui's web tests take ~6 minutes instead of ~74 on master.
dart-test --package-tagsdart-test --package-tags (currently just material_ui web tests)
There was a problem hiding this comment.
Code Review
This pull request updates the dart-test command in the repository tooling to optimize running tests with package tags. Instead of loading and compiling all test files to filter by tag, the tool now scans and runs only the specific test files that literally contain the tag name, falling back to the full test suite filter if tag selector expressions or certain dart_test.yaml configurations are used. Feedback on the changes suggests checking if the test directory exists before listing its contents in _testFilesForTag to prevent a potential FileSystemException crash on packages without a test directory.
QuncCccccc
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the improvement!
mdebbar
left a comment
There was a problem hiding this comment.
Nothing wrong with the PR, I just want to try a fix upstream before we commit to a workaround.
Workaround for flutter/flutter#195700
Part of flutter/flutter#192308
When a package has an entry in
--package-tags(currently justmaterial_ui, for the web unit tests),dart-testnow passesflutter test(ordart test) only the*_test.dartfiles that contain the tag, instead of the wholetest/directory. Before, every test file was compiled and loaded just to find out whether it had any tagged tests. Now only the files that mention the tag are compiled and loaded.This matters most on master, where flutter/flutter#193485 roughly doubled how long each web test suite takes to load. That pushed the shard running the
material_uiweb tests past its 90-minute timeout.With this change,
material_uiloads 52 test files on web instead of 540, and its web tests are about 13× faster.material_ui,example,test_apps/a11y_assessments)material_uiweb test time, mastermaterial_uiweb test time, stable (#26867 → #26871)Caveats
1. The tag must appear literally in the test file
Test files are chosen by searching their text for the tag name. So the tag must be written out in each test file that uses it.
This works
This does not
To catch this,
dart-testnow fails a tagged package if the tag appears in any non-test Dart file in itslib/ortest/directories, and lists the offending files.2. Falls back to old behavior (load/compile all test files) if it's not safe to only load/compile the tagged files
In certain cases, a simple string search isn't sufficient to find all tagged tests. In that case, we fallback to loading and compiling all test files. This happens when:
!slow,a || b);dart_test.yamlcontains any ofadd_tags,paths,filenameorincludekeys.Finally, a tagged package with no test files containing the tag is now skipped with "No test files contain the requested tag." instead of starting
flutter test. For example,material_ui/test_apps/a11y_assessmentsused to load 45 suites and run 0 tests.Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini 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.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2