- Notifications
You must be signed in to change notification settings - Fork1.7k
Kotlin: tweak plugin test#20039
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Put less emphasis on plugin build isolation, to get a better DevEx outof it. The crux of the test is the database extraction part, not theplugin build.
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 modifies the Kotlin custom plugin integration test to improve developer experience by reducing build isolation emphasis and reusing cached build artifacts. The focus shifts from isolated plugin builds to the core database extraction functionality.
Key changes:
- Switches plugin build to run from internal repository with cached artifacts
- Updates build target reference and working directory
- Adjusts resource path handling for the new build context
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test.py | Removes build isolation flags, changes target reference to external repo format, and updates working directory |
| BUILD.bazel | Updates resource strip prefix to handle external repository structure |
| "@codeql//java/ql/integration-tests/kotlin/linux/custom_plugin/plugin", | ||
| ], | ||
| _cwd=test_dir, | ||
| _cwd=semmle_code_dir, | ||
| ) | ||
| shutil.copy( | ||
| "bazel-bin/java/ql/integration-tests/kotlin/linux/custom_plugin/plugin/plugin.jar", | ||
| f"{semmle_code_dir}/bazel-bin/external/ql+/java/ql/integration-tests/kotlin/linux/custom_plugin/plugin/plugin.jar", |
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.
[nitpick] The hardcoded path string contains repetitive path segments. Consider extracting the common path prefix to a variable to reduce duplication and improve maintainability.
Copilot uses AI. Check for mistakes.
| shutil.copy( | ||
| "bazel-bin/java/ql/integration-tests/kotlin/linux/custom_plugin/plugin/plugin.jar", | ||
| f"{semmle_code_dir}/bazel-bin/external/ql+/java/ql/integration-tests/kotlin/linux/custom_plugin/plugin/plugin.jar", |
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.
This hardcoded path is fragile and contains complex nested directory structure. Consider constructing this path more dynamically or storing it in a variable to improve maintainability.
Copilot uses AI. Check for mistakes.
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.
Looks like it works for me
85d1e06 intomainUh oh!
There was an error while loading.Please reload this page.
Put less emphasis on plugin build isolation, to get a better DevEx out of it. The crux of the test is the database extraction part, not the plugin build.
The plugin build is now done from the internal repo, and reusing whatever cached things are there.