-
Notifications
You must be signed in to change notification settings - Fork 38
fix: Improvements for multi-module gradle builds #235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 fixes several issues encountered when using the neotest-java plugin with multi-module Gradle projects. The changes improve test execution in submodules, enhance error reporting for multiple test failures, optimize directory scanning performance, and fix debug output handling.
Key changes:
- Fixed current working directory (cwd) to use module base directory instead of root for multi-module projects
- Removed single quotes from JUnit test selectors that were preventing test discovery
- Refactored directory scanning from recursive to iterative DFS, significantly improving performance
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/spec_builder_spec.lua | Updated test expectations to reflect new user.dir system property, stderr capture config, unquoted selectors, and module-specific cwd paths |
| lua/neotest-java/util/dir_scan.lua | Replaced complex recursive scanning with simple iterative DFS using a stack, eliminating performance bottleneck |
| lua/neotest-java/model/junit_result.lua | Enhanced failure handling to support multiple failures per test with proper message and stacktrace aggregation |
| lua/neotest-java/core/spec_builder/init.lua | Updated to use module base directory for cwd and report directory generation in multi-module projects |
| lua/neotest-java/core/spec_builder/compiler/classpath_provider.lua | Changed from buffer URI to base directory URI for classpath resolution to fix execution from neotest summary |
| lua/neotest-java/command/junit_command_builder.lua | Removed quotes from test selectors and added user.dir system property to ensure tests run in correct directory |
| lua/neotest-java/build_tool/init.lua | Added debug output streaming to DAP REPL and proper cwd handling for debug test execution |
Comments suppressed due to low confidence (1)
lua/neotest-java/command/junit_command_builder.lua:133
- The JDWP agent is started with
address=0.0.0.0:in-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=0.0.0.0:which exposes the Java debug port on all network interfaces without authentication. If a developer runs debug tests on an untrusted network, an attacker on the same network could attach to the JDWP port and gain arbitrary code execution in the test process. Restrict the JDWP bind address tolocalhost(e.g.127.0.0.1) or make it configurable and default to a loopback-only address to avoid exposing the debug port externally.
if port then
table.insert(junit_command.args, 1, "-Xdebug")
table.insert(
junit_command.args,
1,
"-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=0.0.0.0:" .. port
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @lucaneg, thanks for the good work!
The reason to single quote the selector values was #216. Do you have a concrete example value that does not work for the single quotes? We might have to find a third solution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this changes, should add a test in result_builder_spec.lua with an xml report example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great refactor here!
|
There are many different set of changes on this PR. As a recommendation for the next time, try to open multiple PRs better and some stuff would be merged almost immediately. Nice work! |
Co-authored-by: Luca Negrini <luca.negrinii@gmail.com>
Co-authored-by: Luca Negrini <luca.negrinii@gmail.com>
While trying out the plugin on a multi-module gradle project (https://github.com/lisa-analyzer/lisa), I encountered some errors that are fixed by this pr. Below is a rundown of all commits (each addressing a separate problem):
'a.b.c'was searched instead of classa.b.c-- 0c25eac)