Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 14, 2025
Merged

Kotlin: tweak plugin test #20039

merged 2 commits into from
Jul 14, 2025

Conversation

redsun82
Copy link
Contributor

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.

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.
@Copilot Copilot AI review requested due to automatic review settings July 14, 2025 11:56
@redsun82 redsun82 requested a review from a team as a code owner July 14, 2025 11:56
@github-actions github-actions bot added the Java label Jul 14, 2025
Copy link
Contributor

@Copilot 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 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

Comment on lines 15 to 20
"@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",
Copy link
Preview

Copilot AI Jul 14, 2025

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.

Comment on lines 19 to +20
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",
Copy link
Preview

Copilot AI Jul 14, 2025

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.

Copy link
Contributor

@igfoo igfoo left a 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

@redsun82 redsun82 merged commit 85d1e06 into main Jul 14, 2025
15 checks passed
@redsun82 redsun82 deleted the redsun82/kotlin-plugin-test branch July 14, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants