-
Notifications
You must be signed in to change notification settings - Fork 340
Add support for Coverage in Dart projects #5799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Currently this only supports coverage for the project hosting the tests, we need `--coverage-package` to be implemented in Dart to support capturing it for other projects in the workspace like we do for Flutter. Fixes #5575
72d899c to
ad41073
Compare
|
@codex review |
|
Thanks for the pull request, DanTup! I've reviewed the changes and they look solid. You've done a great job extending the test coverage functionality to Dart projects, which is a significant improvement. Here are my observations:
Overall, this is a well-thought-out and executed change. The |
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
This pull request adds support for collecting code coverage in Dart projects, which was previously only available for Flutter projects. The changes look good and correctly introduce a new capability to detect if the test package version supports coverage. I've found one critical issue where coverage reporting for single-package Flutter projects would be broken, and one medium-severity issue to improve logging. I've provided suggestions for both.
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.
Pull Request Overview
This PR adds support for code coverage collection in Dart test projects, extending the existing Flutter coverage functionality. The implementation is currently limited to capturing coverage for the project hosting the tests, with multi-package workspace support requiring the --coverage-package flag to be implemented in Dart.
Key Changes:
- Added capability detection for LCOV coverage support in Dart's test package (version 1.27.0+)
- Implemented coverage collection for Dart tests using
--coverage-pathand--branch-coverageflags - Updated test infrastructure to verify coverage functionality with new test cases
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/test/test_projects/hello_world/test/empty_test.dart | New test file with reflective and group-based tests for coverage validation |
| src/test/test_projects/hello_world/test/basic_test.dart | Added test case that exercises main application code to generate coverage data |
| src/test/test_projects/hello_world/pubspec.yaml | Updated test package dependency to version 1.27.0 to support coverage features |
| src/test/test_projects/hello_world/example/pubspec.yaml | Updated test dependency and added parent package reference for cross-package testing |
| src/test/test_projects/hello_world/example/lib/printer.dart | Modified to delegate to parent package for cross-package coverage testing |
| src/test/helpers.ts | Added file path constant for printer file used in coverage tests |
| src/test/dart_debug/dart_test.test.ts | Added comprehensive coverage test suite with Sinon stubs to verify collection |
| src/test/dart/utils/test_outline_visitor.test.ts | Updated test count assertion to account for new test case |
| src/shared/utils/test.ts | Extended launch configuration to support coverage for Dart projects alongside Flutter |
| src/shared/test/test_model.ts | Added async coverage support detection per test suite based on project capabilities |
| src/shared/capabilities/dart_test.ts | Added capability checks for LCOV coverage support via version and help text |
| src/extension/test/vs_test_controller.ts | Updated test item tag assignment to include coverage tags based on async capability check |
| src/extension/extension.ts | Updated TestModel constructor to pass workspace context for capability detection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Currently this only supports coverage for the project hosting the tests, we need
--coverage-packageto be implemented in Dart to support capturing it for other projects in the workspace like we do for Flutter.Fixes #5575