Skip to content

fix: resolve dart coverage package_config in pub workspaces - #1689

Open
AzazelSensei wants to merge 1 commit into
VeryGoodOpenSource:mainfrom
AzazelSensei:fix/dart-test-coverage-workspace-package-config
Open

fix: resolve dart coverage package_config in pub workspaces#1689
AzazelSensei wants to merge 1 commit into
VeryGoodOpenSource:mainfrom
AzazelSensei:fix/dart-test-coverage-workspace-package-config

Conversation

@AzazelSensei

Copy link
Copy Markdown

Status

READY

Description

very_good dart test --coverage crashes in a pub workspace because it opens .dart_tool/package_config.json relative to the process cwd. Workspaces only write that file at the root.

I pass the package directory as packagePath so the coverage package uses findPackageConfig and walks up, same as dart.

Fixes #1683

Type of Change

  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Pub workspaces only write .dart_tool/package_config.json at the
workspace root. Pass the package directory as packagePath so the
coverage resolver walks up the same way dart does.
@marcossevilla

Copy link
Copy Markdown
Member

Nice fix, this is a cleaner approach than a hand-rolled walk-up. One test-coverage gap worth closing before merge:

The new test (resolves dart coverage package_config from the pub workspace root) proves the pipeline no longer crashes, but it never proves a package: URI actually resolves through the workspace-root config back to a real source file. The seeded package_config.json has an empty "packages":[], and the coverage/ dir never gets a real coverage JSON (only an empty lcov.info), so HitMap.parseFiles runs with zero entries either way. That's enough to catch the PathNotFoundException regression, but not enough to prove resolution actually works.

To close the gap, the test would need:

  1. A real package entry in the workspace-root package_config.json, e.g.
    {"configVersion":2,"packages":[{"name":"foo","rootUri":"../packages/foo","packageUri":"lib/"}]}
  2. A real source file, e.g. member/lib/foo.dart.
  3. A coverage/*.json file in the shape HitMap.parseFiles actually reads ({"coverage":[...]} per hitmap.dart):
    {"coverage":[{"source":"package:foo/foo.dart","hits":[1,1]}]}

Then assert on the resulting lcov.info content (e.g. contains('SF:lib/foo.dart')) instead of just the exit code, to confirm the resolver actually mapped the package: URI back to the file via the workspace-root config.

Not blocking since none of the other dart-coverage tests in this file do this either (they all use empty/placeholder coverage data), but it's the one thing that would make this test prove the fix rather than just prove no crash.

@marcossevilla

Copy link
Copy Markdown
Member

hey @AzazelSensei, thanks for opening a PR! Will you be able to apply the change I mentioned above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting for response Waiting for follow up

Projects

Status: Community

Development

Successfully merging this pull request may close these issues.

fix: very_good dart test --coverage fails in pub workspaces

2 participants