-
Notifications
You must be signed in to change notification settings - Fork 220
Add --coverage-path
and --branch-coverage
options
#2517
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
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
This comment was marked as resolved.
This comment was marked as resolved.
--coverage-lcov
and --branch-cov
options--coverage-lcov
and --branch-cov
options
--coverage-lcov
and --branch-cov
options--coverage-lcov
and --branch-coverage
options
The remaining test failures don't seem related to this PR |
This is ready for review. @jakemac53 @natebosch PTAL. |
@jakemac53 @natebosch Friendly ping |
Just FYI I am no longer officially on this project and don't have time to do reviews at the moment 😭 . |
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.
I think if we rename --coverage-lcov
to --coverage-path
as mentioned on the issue we'll be pretty well set up to normalize things in the future. It's a bit less obvious in the mean time, but I think it'll be a better place to end up long term.
--coverage-lcov
and --branch-coverage
options--coverage-path
and --branch-coverage
options
Revisions updated by `dart tools/rev_sdk_deps.dart`. dartdoc (https://github.com/dart-lang/dartdoc/compare/414953e..82b48b5): 82b48b53 2025-08-05 István Soós Update highlight.js download instructions and resources. (dart-lang/dartdoc#4081) ecosystem (https://github.com/dart-lang/ecosystem/compare/2fe3618..4543c38): 4543c38 2025-08-06 Devon Carew address a `Bad state: No element` element exception (dart-lang/ecosystem#321) e14472f 2025-08-06 Devon Carew add a roadmap to our various mono-repos (dart-lang/ecosystem#318) e83479a 2025-08-06 Moritz Fixes to health (dart-lang/ecosystem#363) 552f534 2025-08-06 Moritz Update API tool hash (dart-lang/ecosystem#362) i18n (https://github.com/dart-lang/i18n/compare/c45e050..25cdb1b): 25cdb1b4 2025-08-07 Moritz Reenable mac size checks (dart-lang/i18n#1001) shelf (https://github.com/dart-lang/shelf/compare/082d3ac..2a46b4f): 2a46b4f 2025-08-05 Kevin Moore Update dependencies across 3 packages (dart-lang/shelf#479) test (https://github.com/dart-lang/test/compare/5aef971..9354f23): 9354f239 2025-08-07 Liam Appelbe Add `--coverage-path` and `--branch-coverage` options (dart-lang/test#2517) tools (https://github.com/dart-lang/tools/compare/5e977d6..1b52e89): 1b52e89e 2025-08-06 Moritz Configure Gemini code review (dart-lang/tools#2141) web (https://github.com/dart-lang/web/compare/1d5771b..f3c960f): f3c960f 2025-08-06 Nikechukwu [web_generator] Add support for passing files as globs (dart-lang/web#427) f51cc85 2025-08-06 Nikechukwu [interop] Add Support for Namespaces (dart-lang/web#436) webdev (https://github.com/dart-lang/webdev/compare/7ff2d07..94c172c): 94c172cc 2025-08-05 Nicholas Shahan Wait for any remaining output in e2e_test.dart (dart-lang/webdev#2663) Change-Id: Ie18635903f7379bc27e7ae23b916e10e4bdcef94 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444360 Auto-Submit: Devon Carew <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
… webdev" This reverts commit 00240e8. Reason for revert: failures on HHH Original change's description: > [deps] rev dartdoc, ecosystem, i18n, shelf, test, tools, web, webdev > > Revisions updated by `dart tools/rev_sdk_deps.dart`. > > dartdoc (https://github.com/dart-lang/dartdoc/compare/414953e..82b48b5): > 82b48b53 2025-08-05 István Soós Update highlight.js download instructions and resources. (dart-lang/dartdoc#4081) > > ecosystem (https://github.com/dart-lang/ecosystem/compare/2fe3618..4543c38): > 4543c38 2025-08-06 Devon Carew address a `Bad state: No element` element exception (dart-lang/ecosystem#321) > e14472f 2025-08-06 Devon Carew add a roadmap to our various mono-repos (dart-lang/ecosystem#318) > e83479a 2025-08-06 Moritz Fixes to health (dart-lang/ecosystem#363) > 552f534 2025-08-06 Moritz Update API tool hash (dart-lang/ecosystem#362) > > i18n (https://github.com/dart-lang/i18n/compare/c45e050..25cdb1b): > 25cdb1b4 2025-08-07 Moritz Reenable mac size checks (dart-lang/i18n#1001) > > shelf (https://github.com/dart-lang/shelf/compare/082d3ac..2a46b4f): > 2a46b4f 2025-08-05 Kevin Moore Update dependencies across 3 packages (dart-lang/shelf#479) > > test (https://github.com/dart-lang/test/compare/5aef971..9354f23): > 9354f239 2025-08-07 Liam Appelbe Add `--coverage-path` and `--branch-coverage` options (dart-lang/test#2517) > > tools (https://github.com/dart-lang/tools/compare/5e977d6..1b52e89): > 1b52e89e 2025-08-06 Moritz Configure Gemini code review (dart-lang/tools#2141) > > web (https://github.com/dart-lang/web/compare/1d5771b..f3c960f): > f3c960f 2025-08-06 Nikechukwu [web_generator] Add support for passing files as globs (dart-lang/web#427) > f51cc85 2025-08-06 Nikechukwu [interop] Add Support for Namespaces (dart-lang/web#436) > > webdev (https://github.com/dart-lang/webdev/compare/7ff2d07..94c172c): > 94c172cc 2025-08-05 Nicholas Shahan Wait for any remaining output in e2e_test.dart (dart-lang/webdev#2663) > > Change-Id: Ie18635903f7379bc27e7ae23b916e10e4bdcef94 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444360 > Auto-Submit: Devon Carew <[email protected]> > Commit-Queue: Konstantin Shcheglov <[email protected]> > Reviewed-by: Konstantin Shcheglov <[email protected]> No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: I239eff7add9e5432c67b05c8f7731e1a79d4e4db Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444387 Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Rubber Stamper <[email protected]> Auto-Submit: Devon Carew <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]>
This may need some attention before we can publish it. The dependency on |
Can you give some more details? package:coverage doesn't depend on package:test afaik (aside from a dev dep) |
Ah sorry I should have updated this issue - after I dug further I found it wasn't a cycle and we just needed a tweak in the build targets to fix the config around a conditional import. No worries here - I have the roll to google3 working. |
Revisions updated by `dart tools/rev_sdk_deps.dart`. test (https://github.com/dart-lang/test/compare/5aef971..b99d556): b99d556 2025-09-01 dependabot[bot] Bump the github-actions group across 1 directory with 3 updates (dart-lang/test#2534) abe4939b 2025-08-27 Nate Bosch Remove executable argument forwarding in tests (dart-lang/test#2533) 81e0579c 2025-08-26 Ömer Sinan Ağacan Serve dart2wasm source map files (dart-lang/test#2532) 9354f239 2025-08-07 Liam Appelbe Add `--coverage-path` and `--branch-coverage` options (dart-lang/test#2517) Change-Id: I7297cc534d03de343218c829a8e94d70ba35a023 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448423 Auto-Submit: Devon Carew <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
It looks like this PR broke Flutter build (after package:test was rolled to Dart SDK):
|
This reverts commit caece5d. Reason for revert: broke Flutter build (dart-lang/test#2517 (comment)) Original change's description: > [deps] rev test > > Revisions updated by `dart tools/rev_sdk_deps.dart`. > > test (https://github.com/dart-lang/test/compare/5aef971..b99d556): > b99d556 2025-09-01 dependabot[bot] Bump the github-actions group across 1 directory with 3 updates (dart-lang/test#2534) > abe4939b 2025-08-27 Nate Bosch Remove executable argument forwarding in tests (dart-lang/test#2533) > 81e0579c 2025-08-26 Ömer Sinan Ağacan Serve dart2wasm source map files (dart-lang/test#2532) > 9354f239 2025-08-07 Liam Appelbe Add `--coverage-path` and `--branch-coverage` options (dart-lang/test#2517) > > Change-Id: I7297cc534d03de343218c829a8e94d70ba35a023 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448423 > Auto-Submit: Devon Carew <[email protected]> > Reviewed-by: Konstantin Shcheglov <[email protected]> > Commit-Queue: Konstantin Shcheglov <[email protected]> Change-Id: I5404155f0de9330d37ff74741dd64a1a8d0436d2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448906 Reviewed-by: Alexander Aprelev <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
@liamappelbe However, when using a bleeding-edge build based on 44207f801bafa97152a722bc794a22d67719a7e5 I get errors that
My first thought was that the |
Oh, never mind - this is because it depends on the version of |
--coverage-path
option todart test
, which takes a filename. This option works like--coverage
, except that instead of generating multiple JSON reports, it generates a single LCOV report.pkgs/test_core/lib/src/runner/coverage.dart
's coverage collection functions, and howpkgs/test_core/lib/src/runner/engine.dart
invokes them.writeCoverage
is still called, but doesn't write the JSON file. Instead it returns the generated coverage report. TheEngine
merges all these coverage reports into_allCoverageData
, and once all the tests are done it callswriteCoverageLcov
to finalize the report and write it to the lcov file.--branch-coverage
flag that is plumbed through tocollect
'sbranchCoverage
param.--coverage-path
workflow.collect
'sscopedOutput
param to the current package name. This front load the filtering of the coverage report, improving performance by removing unnecessary information as early as possible.runPub
was using the deprecated standalonepub
tool. This didn't cause problems because it was dead code. But I had to use it in the new tests, so I updated it to usedart pub
.Fixes #2511