Skip to content

feat: support additional Java test source paths - #1939

Open
Vinay Kumar Maheshwaram (sudovinay01) wants to merge 11 commits into
microsoft:mainfrom
sudovinay01:feature/additional-test-source-paths
Open

Vinay Kumar Maheshwaram (sudovinay01) wants to merge 11 commits into
microsoft:mainfrom
sudovinay01:feature/additional-test-source-paths

Conversation

@sudovinay01

@sudovinay01 Vinay Kumar Maheshwaram (sudovinay01) commented Sep 26, 2026 •

Copy link
Copy Markdown

Why this is needed

The Java Test Runner currently discovers test files from source roots classified as test roots by the Java project model. In Gradle projects, src/main/java remains a main source root, so TestNG classes there can compile but are not discovered for editor test actions or Test Explorer. Reclassifying that directory as a Gradle test source set would change build semantics and is not appropriate for projects that intentionally keep these classes in main sources.

IntelliJ IDEA and Eclipse already discover tests in both main and test source folders and provide run/debug actions for them. This change brings that IDE-level discovery capability to VS Code as an opt-in, without changing Gradle source-set classification or task behavior.

What this changes

  • Adds the opt-in, workspace-scoped java.test.additionalTestSourcePaths setting. Its default is an empty array, preserving existing behavior.
  • Accepts literal source paths or workspace-relative glob patterns and expands glob matches into source directories. The glob library is declared as a production dependency.
  • Includes resolved roots in source-path checks and file watching, scans existing Java files with bounded concurrency, and applies Test Explorer updates sequentially.
  • Clears cached paths and rebuilds discovery when this setting changes or the user refreshes the Testing view. Overlapping refresh requests are serialized to avoid watcher races.
  • Uses stable workspace URI keys, ignores nonexistent literal directories, reports glob errors in the Test Runner for Java Output channel, and deduplicates roots.
  • Logs a heads-up when patterns match more than 100 directories; no hard limit is imposed.
  • Does not modify Gradle source-set configuration or task behavior.

Configuration example

{
  "java.test.additionalTestSourcePaths": ["app/**/src/main/java"]
}

Reproducer

The manual acceptance project is available at https://github.com/sudovinay01/repro-java-issue.

Validation

  • npm test: 165 extension tests passed; activation test passed.
  • npm run lint passed.
  • Production webpack bundle compiled successfully.
  • Regression tests verify additional roots are watched and overlapping refresh requests are serialized.
  • Manual reproducer check: AppMainSourceTest and AppTest appeared in Test Explorer and both passed (2/2).

@azure-pipelines

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

@sudovinay01

Copy link
Copy Markdown
Author

Vinay Kumar Maheshwaram (Vinay Kumar Maheshwaram (@sudovinay01)) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@sudovinay01

Copy link
Copy Markdown
Author

Could a maintainer please help triage this PR or assign a reviewer when available? It has been open since September 26, and I would appreciate guidance on whether any changes are needed before review. Thank you.

@sudovinay01

Copy link
Copy Markdown
Author

Merged current upstream main into this branch in fcb779f and regenerated package-lock.json from the merged manifest, preserving glob as a production dependency. Validation passed: npm ci, npm run compile, npm run lint, npm run vscode:prepublish, npm run build-plugin, and npm test (165 extension tests plus the unmanaged-folder test). GitHub now reports the PR as mergeable.

@sudovinay01

Copy link
Copy Markdown
Author

Hi Jinbo Wang (@testforstephen) Changyong Gong (@chagong) wenyt (@wenytang-ms) — could one of you please review this PR when you have time? I’d appreciate feedback on the change and validation, and I’d like to make sure it aligns with the project’s expectations. Thanks for your time and review.

@wenytang-ms

Copy link
Copy Markdown
Contributor

Thanks Vinay Kumar Maheshwaram (@sudovinay01) for raising this use case and contributing the implementation! Supporting test discovery in main source roots makes sense without requiring users to change their build configuration. The following design aspects need to be adjusted before this can be merged:

  1. Unify discovery scope: The client includes additional paths, but project-level discovery on the Java backend still uses the original test roots. Subsequent project refreshes could remove tests discovered by the client.
  2. Keep related features consistent: Editor actions, Test Explorer, and "Go to Test" should use the same discovery scope. The current implementation only extends the client-side paths.
  3. Avoid a separate full-file scanning pipeline: Scanning every Java file during activation and refresh adds potentially significant overhead. Concurrency limits do not bound the total work, and cancellation is not supported. This should integrate with backend project-level discovery.
  4. Clarify configuration boundaries: Additional discovery paths should be limited to source roots already included in imported Java projects. The setting should explicitly state that it does not configure source roots, dependencies, or compilation outputs.
  5. Narrow the configuration scope: Arbitrary paths and glob patterns introduce long-term complexity around directory changes, overlapping roots, multi-module projects, and performance. We should first establish whether a simpler option to include existing main source roots is sufficient.

The discovery scope should be defined consistently in the Java backend, rather than maintained through a separate client-side scanning path.

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

Copy link
Copy Markdown
Author

wenyt (@wenytang-ms) Thanks for the detailed feedback. I have updated the PR to address these points: project-level test discovery now runs through the Java backend and includes selected additional source roots, with the same resolved scope used for Test Explorer, editor actions, file watching, and Go to Test. The separate client-side Java-file scan has been removed; configured literals/globs are intersected with source roots from imported Java projects, and the setting description clarifies that discovery does not change source roots or build configuration. I retained glob support for selecting roots in multi-module projects, with duplicate matching roots deduplicated. Validation now includes npm run compile, npm run lint, npm test (170 tests plus the unmanaged-folder test), npm run vscode:prepublish, the Java plugin build, and manual Test Explorer checks (5/5 tests, refresh, file create/change/delete, and unmatched-path refresh). The updated implementation is in commit b0589ee.

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

Copy link
Copy Markdown
Author

Hi Jinbo Wang (@testforstephen) Changyong Gong (@chagong) wenyt (@wenytang-ms), could a code owner please review this PR when you have a chance? I’d appreciate your feedback. Thank you!

Comment thread scripts/buildJdtlsExt.js
Comment on lines +33 to +36
// JDT LS provides core ASM; load the support bundles before JaCoCo imports them.
'org.objectweb.asm.commons_',
'org.objectweb.asm.tree_'
'org.objectweb.asm.tree_',
'org.jacoco.core_'

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.

Problem:
org.objectweb.asm_ was removed from bundleList and contributes.javaExtensions, and custom bundleOrder sorting was added.

  1. Removing org.objectweb.asm_9.10.1.jar breaks the fix from fix: ship org.objectweb.asm bundles to satisfy Jacoco 0.8.14 (#1866) #1872 (Test Runner for Java Extension downloads old JUnit standalone #1866). com.microsoft.java.test.plugin requires org.objectweb.asm;bundle-version="[9.10.1,9.11.0)" (MANIFEST.MF), and redhat.java releases are decoupled from vscode-java-test. If a user's redhat.java ships an older ASM version, the test plugin will fail to resolve at startup.
  2. In JDT LS (BundleUtils.loadBundles), all bundles in javaExtensions are installed first and collected into a HashSet<Bundle> before startBundles is called, so the array order in package.json does not affect OSGi resolution or start order.

Solution:
Please revert the changes to scripts/buildJdtlsExt.js and contributes.javaExtensions in package.json so org.objectweb.asm_9.10.1.jar and the alphabetical .sort() are preserved.

Comment on lines +43 to +52
if (token) {
if (token.isCancellationRequested) {
return Promise.resolve([]);
}
return getSourcePaths([workspaceFolder.uri.toString()], token).then((sourcePaths: ITestSourcePath[]) =>
token.isCancellationRequested
? []
: resolveAdditionalTestSourcePaths(workspaceFolder.uri.fsPath, configuredPaths,
sourcePaths.map((sourcePath: ITestSourcePath) => sourcePath.testSourcePath)));
}

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.

Problem:
In getAdditionalTestSourcePaths(workspaceFolder, token), passing a token bypasses this.additionalTestSourceMapping and always calls getSourcePaths([workspaceFolder.uri.toString()], token). Because resolveTestItemChildren in testController.ts always passes a CancellationToken when resolving project items, expanding a multi-module workspace with N projects triggers N separate vscode.java.test.get.sourcepath requests for the same workspace folder instead of reusing the cache.

Solution:
Reuse this.additionalTestSourceMapping even when token is provided (only evicting the cached promise if the request fails or is cancelled before completion), and check token.isCancellationRequested before/after awaiting the cached promise.

Comment thread package.json
},
"dependencies": {
"fs-extra": "^10.1.0",
"glob": "^13.0.6",

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.

Problem:
glob was moved from devDependencies to production dependencies in package.json only to import hasMagic in src/provider/testSourceProvider.ts, while minimatch is already a direct production dependency.

Solution:
Keep glob in devDependencies and use minimatch directly (e.g., new minimatch.Minimatch(pattern).hasMagic()) instead of importing hasMagic from glob.

Comment on lines +176 to +180
const matchPath: string = path.isAbsolute(normalizedPattern)
? sourcePath.replace(/\\/g, '/')
: path.relative(workspacePath, sourcePath).replace(/\\/g, '/');
if (matchPath !== '..' && !matchPath.startsWith('../') && !path.isAbsolute(matchPath) &&
matcher.match(matchPath)) {

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.

Problem: When normalizedPattern is an absolute glob pattern (path.isAbsolute(normalizedPattern) is true), matchPath is set to sourcePath.replace(/\\/g, '/'), which is an absolute path. Because the if condition unconditionally requires !path.isAbsolute(matchPath), absolute glob patterns will always fail this check and never match any source root.

Solution: Only apply the relative-path boundary check (matchPath !== '..' && !matchPath.startsWith('../') && !path.isAbsolute(matchPath)) when !path.isAbsolute(normalizedPattern), and add a unit test covering absolute glob patterns.

Comment on lines +87 to +107
assertTrue(packages.get(0).getChildren().stream()
.noneMatch(item -> "AdditionalMainSourceTest".equals(item.getLabel())));

final String mainSourcePath = javaProject.getPackageFragmentRoot(project.getFolder("src/main/java"))
.getResource().getLocation().toOSString();
final List<JavaTestItem> packagesWithAdditionalSource = TestSearchUtils.findTestPackagesAndTypes(
Arrays.<Object>asList(javaProject.getHandleIdentifier(), Collections.singletonList(mainSourcePath)),
new NullProgressMonitor());
assertTrue(packagesWithAdditionalSource.get(0).getChildren().stream()
.anyMatch(item -> "AdditionalMainSourceTest".equals(item.getLabel())));
assertTrue(packagesWithAdditionalSource.get(0).getChildren().stream()
.anyMatch(item -> "MiniTest".equals(item.getLabel())));

final String additionalTestUri = javaProject.findType("example.AdditionalMainSource")
.getResource().getLocationURI().toString();
final Object defaultNavigation = TestNavigationUtils.findTestOrTarget(
Arrays.asList(additionalTestUri, true), new NullProgressMonitor());
assertTrue(getNavigationItems(defaultNavigation).isEmpty());
final Object navigationWithAdditionalSource = TestNavigationUtils.findTestOrTarget(Arrays.<Object>asList(
additionalTestUri, true, Collections.singletonList(mainSourcePath)), new NullProgressMonitor());
assertEquals(1, getNavigationItems(navigationWithAdditionalSource).size());

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.

Problem: preview-junit was specifically set up as a plain Eclipse JDT project (javanature only) to test JUnit 5 discovery with older --enable-preview compiler settings. Converting preview-junit into a Maven project (pom.xml + m2e.core.maven2Nature) and appending additionalSourcePaths assertions into testDiscoverJUnit5TestsWithOlderPreviewSource() couples two unrelated test scenarios and risks m2e overriding the custom .classpath / compiler .settings that the preview test relies on.

Solution: Keep the preview-junit fixture and testDiscoverJUnit5TestsWithOlderPreviewSource() unchanged, and add a separate @Test method in TestSearchUtilsTest (using an existing Maven fixture or a dedicated fixture) to test additionalSourcePaths discovery and navigation.

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