Add documentation for overarching tests in flutter, add skill to help agents figure out where tests go#185148
Conversation
… agents figure out where tests go
40c8998 to
6f12bf4
Compare
|
After talking to victoria we are going to collaborate on a skill she has been authoring this pr needs to be updated to only be the documenation. |
There was a problem hiding this comment.
Code Review
This pull request adds a new documentation file, Test-Types-Overview.md, which categorizes and describes the various test types in the Flutter codebase. The review feedback points out several broken relative links to other documentation files and suggests corrections. It also recommends clarifying the repository location for engine tests and adding examples for running modern integration tests.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive inventory of test types within the Flutter codebase, categorizing them by location, runtime length, and purpose. The documentation covers Framework, Tool, Integration, DeviceLab, Benchmarks, Analysis, Package, and Engine tests. Feedback was provided regarding numerous incorrect relative paths for documentation links throughout the file and a suggestion to clarify the repository location for engine tests.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new documentation file, Test-Types-Overview.md, which provides a categorized inventory of tests within the Flutter codebase, including details on their location, execution commands, and performance characteristics. Review feedback points out incorrect relative paths for DeviceLab documentation links and suggests updating the run_tests.py path for engine tests to ensure the commands are accurate for the specified working directory.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| - **Count**: ~500+ files. | ||
|
|
||
| ### 3. Integration Tests | ||
| - **Location**: `dev/integration_tests/` and `packages/integration_test/` |
There was a problem hiding this comment.
packages/integration_test/ isn't actually a test, right, that's a Flutter package and falls under the 7. Package Tests section?
There was a problem hiding this comment.
Dang, you are right. If you like the structure in https://github.com/flutter/flutter/pull/185148/changes#r3102401106 then I think I would remove sub packages except for flutter_tool and describe what test directories exist for any "package" which is defined as a folder with a pubspec.yaml entry.
| - Run an integration test: `bin/flutter test integration_test/foo_test.dart` (Run from the package root). | ||
| - Run a driver test: `flutter drive -t <test> --driver <driver>` | ||
| - Example: `flutter drive -t lib/keyboard_resize.dart --driver test_driver/keyboard_resize_test.dart` (Run from the specific test directory). |
There was a problem hiding this comment.
This is a little tricky because I think dev/integration_tests are meant to be used with devicelab tests and not all of them will have driver or Flutter integration tests. Some are native app tests. Perhaps instead of a separate section on integration tests directory, add a link to the dev/integration_tests/README.md under the Devicelab section?
There was a problem hiding this comment.
Open to suggestions. Part of what this documentation shows is how difficult it is to reason about.
There was a problem hiding this comment.
Yeah, this directory is kind of a catch-all. For example, dev/integration_tests/widget_preview_scaffold is just an inflated version of the widget preview scaffold template so we can write unit tests against it.
Co-authored-by: Victoria Ashworth <[email protected]>
justinmc
left a comment
There was a problem hiding this comment.
LGTM from the framework side 👍 . Good call adding this. If LLMs can reliably set up and run our tests it will be a big help.
There was a problem hiding this comment.
I'm still iffy on putting the commands in this file because I've found that agents have a difficult time figuring out what directory to run the test in. They often assume the root directory, which is not the case for some tests. I fear that it'll be lazy and not read the actual README where the directory information is and just run the command given here.
I haven't tried it out, though, so we can just see how it does and adjust as needed.
Fair feedback, I agree if agents run into this issue then we would need to change something. I am hoping/expecting that skills for how to run tests will be next. |
Add overview documentation for the types of tests in flutter/flutter
AI assisted pull request: Prompt flow and specifics here (googlers only) go/reidbaker-testing-overview-and-skill-working-doc
@vashworth for ios
@jtmcdole for engine and overall ci
@loic-sharma as an fyi because this is super light for desktop
@mdebbar for web
@justinmc for framework
Pre-launch Checklist
///).