Skip to content

feat: delegate-test coverage through BSP (#1890) - #1893

Draft
wenyt (wenytang-ms) wants to merge 11 commits into
developfrom
wenyt/delegate-test-p0
Draft

wenyt (wenytang-ms) wants to merge 11 commits into
developfrom
wenyt/delegate-test-p0

Conversation

@wenytang-ms

@wenytang-ms wenyt (wenytang-ms) commented Jul 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

First opt-in P0 slice of #1890: add Delegate Test to Gradle (Coverage) without changing the user's build files. The profile remains non-default so we can onboard users and observe adoption before changing the default test experience.

Design

BSP-first execution

Coverage uses the existing BSP buildTarget/test path and Tooling API TestLauncher, preserving Gradle's test environment and streamed test results. A per-run init script:

  • applies JaCoCo when the project does not already apply it;
  • writes execution data and XML reports to a unique temporary directory;
  • finalizes each executed Test task with its project's jacocoTestReport.

The build server sends the finish notification after TestLauncher.run() and its finalizers complete. Coverage is attached to the VS Code TestRun before the run ends.

Task-server fallback

When BSP delegation is unavailable, the existing task-server fallback runs cleanTest test --tests ... jacocoTestReport and parses JUnit XML. It uses the same isolated JaCoCo data directory.

Coverage rendering

JaCoCo XML is converted to VS Code FileCoverage, StatementCoverage, and BranchCoverage. Detailed coverage is cached per TestRun.

Source resolution

The JDTLS command java.gradle.getBuildTargetInfo returns:

  • the project's Gradle version;
  • BSP source roots with test/generated metadata.

Coverage prefers production, non-generated roots and falls back to a workspace glob when BSP metadata is unavailable.

Compatibility and concurrency

  • Coverage requires Gradle 6.1 or newer. Known older versions are rejected with a clear error before dispatch.
  • Delegated runs are serialized because current BSP callbacks do not contain a run id. The previous idle-timeout release was removed because elapsed silence cannot safely distinguish a dropped connection from a long-running test.

Onboarding

This PR intentionally adds an explicit profile rather than failure-driven automatic retry. Existing java.test.runTests telemetry records the selected profile label, so adoption can be measured without making the profile default.

Validation

  • TypeScript compile and lint
  • Extension unit suite: 118 passing, 1 pending
  • JDTLS plugin package build
  • Manual JUnit 5 coverage run

Follow-ups

  • Thread BSP originId through test callbacks to support per-run routing and safe concurrent runs.
  • Evaluate making delegation the project-level default after onboarding data and reliability feedback.
  • Consider BSP-native coverage result transport to remove client-side XML parsing.

P0(A) Coverage under delegate: register a Coverage run profile and, on
coverage runs, transparently inject JaCoCo into every Test task via the
existing per-run init script, run jacocoTestReport, and translate the
XML report into VS Code FileCoverage/StatementCoverage - no build.gradle
edits required.

P0(B) Failure-driven escalation: classify delegation-fixable failures
(JPMS module-access errors, stale/unprocessed resources) and offer a
one-click "Retry with Gradle" action.

Bumps the minimum VS Code engine to 1.88 for the stable coverage API.

Ref: #1890
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

…egate coverage

- Parse JaCoCo cb/mb counters into VS Code BranchCoverage on statement lines

- Disambiguate source file URIs across modules, preferring src/main then src

- Update and extend delegate P0 unit tests for branch coverage
- Set toolVersion = 0.8.15 when we apply the jacoco plugin ourselves, so instrumentation succeeds on recent JDKs instead of inheriting Gradle's older bundled default

- Leave projects that already manage their own JaCoCo version untouched
@wenytang-ms

Copy link
Copy Markdown
Contributor Author
delegatetogradle.mp4

Coverage now runs the tests through the BSP delegate (TestLauncher —
faithful execution + streamed results) with a JaCoCo init script that
instruments the test JVM and wires `test.finalizedBy(jacocoTestReport)`,
so the report is produced in the same BSP invocation instead of a
task-server-only `cleanTest test ... jacocoTestReport`. The task-server
path (launchViaTaskServer, renamed from launchXmlFallback) remains the
fallback when BSP is unavailable.

Coverage source files are resolved against authoritative BSP source
roots via a new `java.gradle.getBuildTargetSources` jdtls delegate
command (buildTarget/sources), with a client-side timeout and Java-side
bounded get(); falls back to a workspace glob otherwise. Fixes wrong-file
mapping in multi-module builds.

Delegated runs are serialized (runChain/releaseGate) so overlapping runs
can't cross the singleton's shared state; an idle watchdog reset on each
per-test update releases the gate only after a run's result stream goes
silent (dropped connection), never during an active run.

Also: remove the failure-driven "Retry with Gradle" escalation (keep the
explicit Coverage run profile); isolate loadDetailedCoverage per TestRun;
extract shared escapeGroovySingleQuoted util.
@wenytang-ms wenyt (wenytang-ms) changed the title feat(P0): delegate-test coverage + Retry with Gradle (#1890) feat(P0): delegate-test coverage through BSP (#1890) Jul 9, 2026
Isolate JaCoCo data per run, gate unsupported Gradle versions, preserve source-root metadata, and remove the unsafe idle serialization release.

Copilot-Session: abc1ed82-1a09-4e77-8d79-88943299db25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Implements the first P0 slice of “Delegate Test to Gradle” default-level test experience by adding coverage support to the delegated Gradle test runner, primarily through BSP (with a task-server fallback). This extends the existing delegated Run/Debug execution path to also support Coverage runs without requiring user build-script changes.

Changes:

  • Register a new Delegate Test to Gradle (Coverage) run profile and add coverage-aware execution paths.
  • Add JaCoCo init-script generation + XML parsing to surface vscode.FileCoverage/detail coverage (including branch counters).
  • Add a new JDTLS delegate command to fetch build target source roots (and Gradle version) for more accurate source mapping.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
extension/src/test/unit/GradleTestRunner.test.ts Adds unit coverage for BSP coverage runs, Gradle version gating, and run serialization behavior.
extension/src/test/unit/delegateTestP0.test.ts Adds unit tests for JaCoCo XML parsing and source match selection logic.
extension/src/Extension.ts Registers the Coverage-kind delegate test run profile.
extension/src/bs/groovy.ts Adds a shared Groovy single-quote escaping helper for init-script generation.
extension/src/bs/GradleTestRunner.ts Adds coverage orchestration (BSP + fallback), per-run coverage collection, and serialization gate for delegated runs.
extension/src/bs/coverage.ts Introduces JaCoCo init-script generation, XML parsing, source resolution, and VS Code coverage attachment/detail caching.
extension/package.json Bumps VS Code engine + @types/vscode to support the coverage APIs/profile kind.
extension/jdtls.ext/.../GradleDelegateCommandHandler.java Implements the new delegate command that returns Gradle version + BSP source roots.
extension/jdtls.ext/.../plugin.xml Registers the new delegate command id.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread extension/jdtls.ext/com.microsoft.gradle.bs.importer/plugin.xml
Use the same five-second bound on both sides of the JDTLS command.

Copilot-Session: abc1ed82-1a09-4e77-8d79-88943299db25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Comment thread extension/src/bs/GradleTestRunner.ts
Comment thread extension/src/test/unit/GradleTestRunner.test.ts
Comment thread extension/src/test/unit/GradleTestRunner.test.ts
Comment thread extension/src/Extension.ts Outdated
Handle prerelease Gradle versions correctly and remove timer-based waits from serialization tests.

Copilot-Session: abc1ed82-1a09-4e77-8d79-88943299db25
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

@wenytang-ms wenyt (wenytang-ms) changed the title feat(P0): delegate-test coverage through BSP (#1890) feat: delegate-test coverage through BSP (#1890) Jul 10, 2026
Comment thread extension/src/bs/coverage.ts Outdated
" }",
" def coverageProjectId = p.path.replaceAll('[^A-Za-z0-9]', '_') + '_' + Integer.toHexString(p.path.hashCode())",
` def coverageExecDir = new File('${executionDataDir}', coverageProjectId)`,
" p.tasks.matching { it.name == 'jacocoTestReport' }.configureEach { r ->",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This reconfigures the user's existing jacocoTestReport task, overriding its execution data and report outputs. It can also trigger user-defined dependencies or actions when delegated coverage runs. Could we register a uniquely named report task owned by the extension and finalize the selected Test tasks with that instead? This would keep the coverage run isolated from the user's JaCoCo configuration.

@wenytang-ms

Copy link
Copy Markdown
Contributor Author

Revised convergence: land the exec-only, JDTLS-analyzed coverage model

Updating my earlier A/B/C convergence comment on #1890. After tracing the actual code paths, the conclusion changes:

Once coverage analysis is owned by JDTLS, no report task is needed, and a pure BSP buildTarget/test + init-script path is sufficient — no protocol change, no two-launcher bridge.

The .exec-boundary model in the #1890 body therefore strictly dominates options A/B/C, and this PR should converge on it rather than on B.

1. The "coverage needs a report task" premise was wrong

My earlier comment claimed coverage "inherently needs a report task that TestLauncher won't run, so any BSP-native solution must bridge the two launchers." That does not hold under the body's own design:

  • JDTLS's CoverageHandler loads .exec and analyzes against the IJavaProject output locations. We never run jacocoTestReport.
  • No report task ⇒ no BuildLauncher.forTasks ⇒ nothing to bridge.
  • TestLauncher (BSP buildTarget/test) + a per-run --init-script that only attaches the JaCoCo agent and redirects its destfile into the run directory is the entire execution path — fully on BSP, zero protocol change.

Consequently:

Target = the #1890 body model. This PR converges there, not on B.

2. Concrete changes for this PR (half-baked B → exec-only)

Today this PR rides BSP but still reconfigures and finalizedBy-runs jacocoTestReport to produce XML, then parses that XML client-side. To reach the target:

  • Delete client-side XML handling in coverage.ts: the jacocoTestReport reconfiguration in getCoverageInitScriptLines (the finalizedBy block), plus parseJacocoXml, resolveSourceFileUri / pickBestSourceMatch, and collectCoverage.
  • Init-script only attaches JaCoCo: set the agent destination into the run's exec dir; never touch jacocoTestReport.
  • Emit isolated .exec per run / Gradle project / Test task into the directory Java Test provides.
  • Drop the task-server coverage invocation; keep the task-server path strictly as the non-coverage fallback.
  • Prefer a raw -javaagent on the Test JVM over JacocoTaskExtension, so coverage does not require the user to apply the jacoco plugin and the Gradle 6.1 report-DSL requirement can be dropped. (Validate agent attachment on TestLauncher.run().)

3. Cross-repo contract (vscode-java-test)

The analysis seam already runs for delegated runs — provideFileCoverage is invoked after the external runner in testController.ts, independent of who launched the JVM. Remaining work there:

  • Multi-.exec merge (blocking). CoverageHandler hard-codes a single jacoco.exec (JACOCO_EXEC). It must accept a directory / file list and call ExecFileLoader.load(...) repeatedly. This is now the primary path, not an edge case.
  • Coverage artifact contract. Add the run-scoped descriptor to IRunTestContext (coverage?: { format: "jacoco-exec"; outputDirectory }), plus the fork-level file/naming strategy (single append destfile vs per-fork files the handler merges).
  • Agent/analyzer version ownership. The agent version must be injected by the integration and pinned to JDTLS's bundled org.jacoco.core, not taken from the user's jacoco plugin. Define behavior on IncompatibleExecDataVersionException. This becomes a hard prerequisite for any future default-on (P2).

4. Make three invariants explicit in the design

These are currently implicit or buried in the validation table, but they are the make-or-break assumptions:

  1. JDT output location == Gradle class output. The importer sets each source entry's output to Gradle's build/classes via buildTargetOutputPaths (GradleBuildServerBuildSupport), which is exactly why JaCoCo class-id matching works. State it as an invariant: if a future importer change breaks the alignment, coverage silently goes empty.
  2. Analysis still requires the JDT project model. We move execution to Gradle but analysis stays JDTLS-coupled; the project must be imported.
  3. No clean between run and analysis. Build-dir classes are marked optional (cleanable); analyzing after a clean yields empty coverage.

5. Reliability / completeness

  • Detect silent-empty coverage. A class-id mismatch doesn't throw — the analyzer just returns nothing. Add a sanity check: if the .exec contains execution data but the analyzer matches zero classes across all output dirs, emit a distinct diagnostic and a dedicated telemetry reason (don't surface it as "0% covered").
  • Escalation stance. Resolve the contradiction between the body's "do not auto-retry a failed standard run with Gradle" and the escalation.ts classifier: make it an explicit, manual one-click "Retry with Gradle", or drop it. No inert half-state.
  • Debug + Coverage. Decide whether coverage-while-debugging is in scope; if so, validate two agents (jdwp + jacoco) coexisting on the forked Test JVM.

Net: execution is ~90% there (BSP-first is right). The change is to stop producing XML and hand .exec to JDTLS, and to land the CoverageHandler multi-exec + artifact contract on the Java Test side. That removes the two-path divergence, keeps everything on TestLauncher, and needs no BSP protocol change.

Delegate-test coverage previously generated and parsed a JaCoCo XML
report on the client. Switch to an exec-only model: Gradle (via the BSP
TestLauncher, or the task-server fallback) attaches the JaCoCo agent and
writes .exec files into the java-test-provided output directory, and
JDTLS' CoverageHandler loads/merges and analyzes them. This removes
client-side XML generation/parsing and keeps a single source of truth for
coverage analysis, matching how the default (non-delegated) runner works.

- coverage.ts: slim to getCoverageInitScriptLines() (JaCoCo agent only,
  no report task, no XML); write per-project/per-task .exec files.
- GradleTestRunner.ts: write .exec to context.coverage.outputDirectory;
  drop client-side collection and the report task in the fallback path;
  defer init-script cleanup to finishTestRun since the delegate command
  dispatches the BSP test run asynchronously and returns before Gradle
  reads the script.
- java-test-runner.api.ts: add coverage.outputDirectory to IRunTestContext.
- Tests updated for the exec-only helper and run context.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8b7a64f2-3b9f-4328-a173-f4728a1fa95c
wenyt (wenytang-ms) and others added 2 commits July 27, 2026 10:42
The directory is specific to the runner that produced the execution data:
JDT-LS merges every `.exec` file below it, so the delegated runner and the
built-in runner each get their own directory.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8b7a64f2-3b9f-4328-a173-f4728a1fa95c
Delegated coverage needs the Test Runner for Java extension to supply an output directory and to analyze the .exec files the Gradle test JVM writes there. Both halves ship as separate extensions, so a user can easily end up on a host that predates the feature, in which case the profile appears in the UI but every run fails.

Feature-detect the capability the host advertises and keep the profile hidden until it is present. The runtime guard inside the runner stays as a second line of defence.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8b7a64f2-3b9f-4328-a173-f4728a1fa95c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants