[coverage] Partial workspace support #2095
Conversation
github-actions
bot
added
type-infra
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthBreaking changes ✔️
Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs.
Coverage
|
| File | Coverage |
|---|---|
| pkgs/coverage/bin/test_with_coverage.dart | 💔 32 % ⬇️ 14 % |
| pkgs/coverage/lib/src/collect.dart | 💔 87 % ⬇️ 0 % |
| pkgs/coverage/lib/src/coverage_options.dart | 💚 100 % |
| pkgs/coverage/lib/src/formatter.dart | 💚 97 % |
| pkgs/coverage/lib/src/hitmap.dart | 💚 91 % ⬆️ 0 % |
| pkgs/coverage/lib/src/util.dart | 💚 100 % ⬆️ 2 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
| Package | Leaked API symbols |
|---|
License Headers ⚠️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
| Files |
|---|
| pkgs/coverage/lib/src/coverage_options.dart |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/bazel_worker/benchmark/benchmark.dart |
| pkgs/bazel_worker/example/client.dart |
| pkgs/bazel_worker/example/worker.dart |
| pkgs/benchmark_harness/integration_test/perf_benchmark_test.dart |
| pkgs/boolean_selector/example/example.dart |
| pkgs/clock/lib/clock.dart |
| pkgs/clock/lib/src/clock.dart |
| pkgs/clock/lib/src/default.dart |
| pkgs/clock/lib/src/stopwatch.dart |
| pkgs/clock/lib/src/utils.dart |
| pkgs/clock/test/clock_test.dart |
| pkgs/clock/test/default_test.dart |
| pkgs/clock/test/stopwatch_test.dart |
| pkgs/clock/test/utils.dart |
| pkgs/html/example/main.dart |
| pkgs/html/lib/dom.dart |
| pkgs/html/lib/dom_parsing.dart |
| pkgs/html/lib/html_escape.dart |
| pkgs/html/lib/parser.dart |
| pkgs/html/lib/src/constants.dart |
| pkgs/html/lib/src/encoding_parser.dart |
| pkgs/html/lib/src/html_input_stream.dart |
| pkgs/html/lib/src/list_proxy.dart |
| pkgs/html/lib/src/query_selector.dart |
| pkgs/html/lib/src/token.dart |
| pkgs/html/lib/src/tokenizer.dart |
| pkgs/html/lib/src/treebuilder.dart |
| pkgs/html/lib/src/utils.dart |
| pkgs/html/test/dom_test.dart |
| pkgs/html/test/parser_feature_test.dart |
| pkgs/html/test/parser_test.dart |
| pkgs/html/test/query_selector_test.dart |
| pkgs/html/test/selectors/level1_baseline_test.dart |
| pkgs/html/test/selectors/level1_lib.dart |
| pkgs/html/test/selectors/selectors.dart |
| pkgs/html/test/support.dart |
| pkgs/html/test/tokenizer_test.dart |
| pkgs/html/test/trie_test.dart |
| pkgs/html/tool/generate_trie.dart |
| pkgs/pubspec_parse/test/git_uri_test.dart |
| pkgs/stack_trace/example/example.dart |
| pkgs/watcher/test/custom_watcher_factory_test.dart |
| pkgs/yaml_edit/example/example.dart |
This check can be disabled by tagging the PR with skip-license-check.
liamappelbe
changed the title
Pull Request Test Coverage Report for Build 15153463985Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| @@ -93,13 +86,13 @@ ArgParser _createArgParser(CoverageOptions defaultOptions) => ArgParser() | |||
| defaultsTo: defaultOptions.scopeOutput, | |||
| help: 'restrict coverage results so that only scripts that start with ' | |||
| 'the provided package path are considered. Defaults to the name of ' | |||
| 'the package under test.') | |||
| 'the current package (including all subpackages, if this is a ' | |||
There was a problem hiding this comment.
I'm assuming one can still specify which subpackages one want coverage for with --scope-output manually?
E.g.
- pkgs/
- my_package/
- lib/ <-- I want coverage for this.
- test_data/
- my_test_package/
- lib/ <-- I don't want coverage for this
- my_test_package/
- my_package/
If I use dart pub global run coverage:test_with_coverage --scope-output my_package -- pkgs/my_package/test will that do what I want?
The test projects are usually part of the workspace w.r.t. analysis. However, they are not part of the packages we want code coverage stats for.
(I know it's not what this PR is doing, because it's automating the --scope-output if not provided. But maybe we can add a test for this use case. Fine to do that in a separate PR.)
There was a problem hiding this comment.
The scoping is based on the package URI:
package:my_package/my_package.dart
\________/
scope
my_test_package is a separate package with a different URI, so dart pub global run coverage:test_with_coverage -- pkgs/my_package/test should work. But yeah, you can also provide the scopes manually if you want.
| @@ -218,8 +208,9 @@ Future<void> main(List<String> arguments) async { | |||
| ); | |||
| final serviceUri = await serviceUriCompleter.future; | |||
|
|
|||
| final scopes = | |||
| flags.scopeOutput.isEmpty ? [flags.packageName] : flags.scopeOutput; | |||
| final scopes = flags.scopeOutput.isEmpty | |||
There was a problem hiding this comment.
This completely removes the packageName rather than deprecating it right? Better to fully remove it then if it's not supported anymore?
There was a problem hiding this comment.
That would be a breaking change.
| - Partial support for workspace packages in `test_wth_coverage`. | ||
| - Deprecate `test_wth_coverage`'s `--package-name` flag, because it doesn't make | ||
| sense for workspaces. | ||
| - Change the default `--port` to 0, allowing the VM to choose a free port. |
There was a problem hiding this comment.
Is there any possibility that a user will be assuming that 8181 is being used by default? If the VM service from the spawned process is something we expect users to want to connect to, this is a breaking change in behavior.
There was a problem hiding this comment.
I considered that, but it seems pretty unlikely. I'd expect people to be manually specifying the port if they need that sort of thing.
| @@ -15,6 +15,7 @@ dependencies: | |||
| meta: ^1.0.2 | |||
| package_config: ^2.0.0 | |||
| path: ^1.8.0 | |||
| pubspec_parse: ^1.5.0 | |||
There was a problem hiding this comment.
Can you switch to using package:yaml? This package has a dep on checked_yaml, which is not available in the sdk repo.
There was a problem hiding this comment.
When I switch to package:yaml, I'm still pulling in pubpsec_parse as an indirect dep:
build_runner -> build_config -> pubpsec_parse
This indirect dep has been around for a long time, but I guess that hasn't been an issue before because build_runner is a dev dependency?
There was a problem hiding this comment.
That's likely it - I think we're just being affected by the non-dev dependencies when trying to resolve in the sdk.
The eventual goal is that the user should be able to run
dart run coverage:test_with_coveragefrom the workspace root, and it runs all the tests in the workspace and produces a single coverage report. There are 2 problems that need to be solved for this to happen:I'm only tackling the scoping changes for now (see this comment). So this means that users can run
test_with_coveragefrom the workspace root, but they have to manually specify the tests they want to run:dart run coverage:test_with_coverage -- pkgs/foo/test pkgs/bar/test.The packages included in the final report are controlled by the
--scope-outputoption. The problem is with the default behavior, where it defaults to the current package. This excludes all the subpackages. The fix is just to parse the pubspec.yaml file, and include all the subpackages in the default scopes. Workspaces can be nested, so we have to do this recursively.#2083
Other changes
--package-nameoption. It's only used to set the--scope-outputoption, so it's a bit redundant. And it doesn't make sense in the context of workspaces anyway.--portoption to 0. There's special logic in the Dart VM that will choose an arbitrary free port if you set--enable-vm-service=0.dart test -x integrationto run all the fast tests during development. Also, pull out the common bits of integration test config to dart_test.yaml.lib/src/collect.dartandlib/src/formatter.dartto fix some failing tests