Skip to content

Conversation

@DanTup
Copy link
Member

@DanTup DanTup commented Aug 5, 2025

Fixes #5623

@DanTup DanTup added this to the v3.118.0 milestone Aug 5, 2025
@DanTup DanTup added is enhancement in flutter Relates to running Flutter apps in testing Relates to test execution of Dart/Flutter tests for end users labels Aug 5, 2025
@DanTup DanTup changed the title Add support for collecting coverage from dependencies that exist in t… Add support for collecting coverage from dependencies that exist in the workspace Aug 5, 2025
@DanTup DanTup requested a review from Copilot August 5, 2025 11:59
@DanTup
Copy link
Member Author

DanTup commented Aug 5, 2025

/gemini review

Copy link

Copilot AI left a 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 collecting code coverage from workspace dependencies when running Flutter tests. It modifies the test runner to include coverage data from packages that exist in the workspace, not just the current package being tested.

Key changes:

  • Extends the test launch configuration to accept workspace package names for coverage collection
  • Adds new test files demonstrating the dependency coverage functionality
  • Updates the test controller to expose the coverage parser for accessing collected coverage data

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/test_projects/flutter_hello_world/lib/printer.dart Adds a simple print function to serve as a dependency for coverage testing
src/test/test_projects/flutter_hello_world/example/test/printer_test.dart Test file that imports and tests the printer function from the parent package
src/test/test_projects/flutter_hello_world/example/pubspec.yaml Configuration for the example package with dependency on the parent package
src/test/test_projects/flutter_hello_world/example/lib/printer.dart Wrapper function that calls the parent package's printer function
src/test/helpers.ts Adds file path constants for the new test project files
src/test/flutter_test_debug/debug/flutter_test.test.ts Adds test coverage verification for dependency packages
src/shared/vscode/interfaces.ts Exposes the coverage parser in the test controller API
src/shared/utils/test.ts Extends launch configuration to include workspace package names for coverage
src/extension/commands/test.ts Implements workspace package discovery for coverage collection

let workspacePackageNames: string[] | undefined;
if (includeCoverage && isFlutter) {
const workspacePackagePaths = await getAllProjectFolders(this.logger, getExcludedFolders, { requirePubspec: true, searchDepth: config.projectSearchDepth });
workspacePackageNames = workspacePackagePaths.map((packagePath) => path.basename(packagePath));
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using path.basename() to extract package names from paths may not be reliable. Consider reading the package name from each package's pubspec.yaml file instead, as the directory name might not always match the actual package name declared in pubspec.yaml.

Copilot uses AI. Check for mistakes.
Copy link

@gemini-code-assist gemini-code-assist bot left a 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 test coverage from workspace dependencies in Flutter projects. The changes correctly identify workspace packages and pass them to the flutter test command using the --coverage-package flag. The implementation is accompanied by good test coverage, including a new test case that verifies coverage is collected from a local dependency.

My review includes a couple of suggestions to improve code conciseness and reduce duplication in the tests. Overall, the changes are solid and address the intended issue effectively.

Comment on lines 30 to 33
for (const packageName of workspacePackageNames) {
toolArgs.push("--coverage-package");
toolArgs.push(packageName);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This loop can be written more concisely using flatMap to create the arguments array in a single expression. This improves readability and is more idiomatic for functional-style programming.

toolArgs.push(...workspacePackageNames.flatMap((packageName) => ["--coverage-package", packageName]));

Comment on lines 500 to 550
describe("collects coverage", () => {
it("for a basic test", async () => {
// Discover tests.
await openFile(flutterTestMainFile);
await waitForResult(() => !!privateApi.fileTracker.getOutlineFor(flutterTestMainFile));
const controller = privateApi.testController;

const suiteNode = controller.controller.items.get(`SUITE:${fsPath(flutterTestMainFile)}`)!;
const testRequest = new vs.TestRunRequest([suiteNode]);

const createTestRunOriginal = controller.controller.createTestRun;
const createTestRunStub = sb.stub(controller.controller, "createTestRun");
let addCoverageStub: SinonStub | undefined;
createTestRunStub.callsFake((request) => {
const originalResult = createTestRunOriginal.call(controller.controller, request);
addCoverageStub = sb.stub(originalResult, "addCoverage").returns(null);
return originalResult;
});
await controller.runTests(false, true, testRequest, fakeCancellationToken);

assert(addCoverageStub?.calledOnce);
const coverage = addCoverageStub.firstCall.args[0] as DartFileCoverage;
assert.equal(fsPath(coverage.uri), fsPath(flutterHelloWorldMainFile)); // App file, not test file.
assert.ok(coverage.statementCoverage.covered > 0);
assert.ok(coverage.statementCoverage.total > 0);
});
await controller.runTests(false, true, testRequest, fakeCancellationToken);

assert(addCoverageStub?.calledOnce);
const coverage = addCoverageStub.firstCall.args[0] as DartFileCoverage;
assert.equal(fsPath(coverage.uri), fsPath(flutterHelloWorldMainFile)); // App file, not test file.
assert.ok(coverage.statementCoverage.covered > 0);
assert.ok(coverage.statementCoverage.total > 0);
it("and includes dependencies", async () => {
// Discover tests.
await openFile(flutterHelloWorldExampleTestFile);
await waitForResult(() => !!privateApi.fileTracker.getOutlineFor(flutterHelloWorldExampleTestFile));

const controller = privateApi.testController;
const suiteNode = controller.controller.items.get(`SUITE:${fsPath(flutterHelloWorldExampleTestFile)}`)!;
const testRequest = new vs.TestRunRequest([suiteNode]);

const createTestRunOriginal = controller.controller.createTestRun;
const createTestRunStub = sb.stub(controller.controller, "createTestRun");
let addCoverageStub: SinonStub | undefined;
createTestRunStub.callsFake((request) => {
const originalResult = createTestRunOriginal.call(controller.controller, request);
addCoverageStub = sb.stub(originalResult, "addCoverage").returns(null);
return originalResult;
});
await controller.runTests(false, true, testRequest, fakeCancellationToken);

assert(addCoverageStub?.called);
const coverageFiles = addCoverageStub.getCalls().map((call) => fsPath((call.args[0] as DartFileCoverage).uri));
assert.deepStrictEqual(coverageFiles, [fsPath(flutterHelloWorldExamplePrinterFile), fsPath(flutterHelloWorldPrinterFile)]);
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's some duplicated test setup code in this describe block. You can extract the common logic for stubbing createTestRun and addCoverage into a beforeEach hook to reduce duplication and improve maintainability.

describe("collects coverage", () => {
		let addCoverageStub: SinonStub | undefined;

		beforeEach(() => {
			const controller = privateApi.testController;
			const createTestRunOriginal = controller.controller.createTestRun;
			const createTestRunStub = sb.stub(controller.controller, "createTestRun");
			createTestRunStub.callsFake((request) => {
				const originalResult = createTestRunOriginal.call(controller.controller, request);
				addCoverageStub = sb.stub(originalResult, "addCoverage").returns(null);
				return originalResult;
			});
		});

		it("for a basic test", async () => {
			// Discover tests.
			await openFile(flutterTestMainFile);
			await waitForResult(() => !!privateApi.fileTracker.getOutlineFor(flutterTestMainFile));
			const controller = privateApi.testController;

			const suiteNode = controller.controller.items.get(`SUITE:${fsPath(flutterTestMainFile)}`)!;
			const testRequest = new vs.TestRunRequest([suiteNode]);

			await controller.runTests(false, true, testRequest, fakeCancellationToken);

			assert(addCoverageStub?.calledOnce);
			const coverage = addCoverageStub!.firstCall.args[0] as DartFileCoverage;
			assert.equal(fsPath(coverage.uri), fsPath(flutterHelloWorldMainFile)); // App file, not test file.
			assert.ok(coverage.statementCoverage.covered > 0);
			assert.ok(coverage.statementCoverage.total > 0);
		});

		it("and includes dependencies", async () => {
			// Discover tests.
			await openFile(flutterHelloWorldExampleTestFile);
			await waitForResult(() => !!privateApi.fileTracker.getOutlineFor(flutterHelloWorldExampleTestFile));

			const controller = privateApi.testController;
			const suiteNode = controller.controller.items.get(`SUITE:${fsPath(flutterHelloWorldExampleTestFile)}`)!;
			const testRequest = new vs.TestRunRequest([suiteNode]);

			await controller.runTests(false, true, testRequest, fakeCancellationToken);

			assert(addCoverageStub?.called);
			const coverageFiles = addCoverageStub!.getCalls().map((call) => fsPath((call.args[0] as DartFileCoverage).uri));
			assert.deepStrictEqual(coverageFiles, [fsPath(flutterHelloWorldExamplePrinterFile), fsPath(flutterHelloWorldPrinterFile)]);
		});
	});

@DanTup DanTup merged commit d0d8c2f into master Aug 5, 2025
21 checks passed
@DanTup DanTup deleted the handle-coverage-for-dependencies branch August 5, 2025 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in flutter Relates to running Flutter apps in testing Relates to test execution of Dart/Flutter tests for end users is enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Include Code Coverage for dependencies in the workspace, not only the main app

2 participants