-
-
Notifications
You must be signed in to change notification settings - Fork 23
Add IntelliJ plugin & module tests #2
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
Conversation
WalkthroughAdds two GitHub Actions workflows (IntelliJ plugin CI and module tests) configuring JDK 21 (Zulu), Gradle caching, and artifact/test-result uploads; extends the Gradle version catalog with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer/Push
participant GH as GitHub Actions
participant Runner as Runner (ubuntu, JDK 21 Zulu)
participant Gradle as Gradle (with cache)
participant IDE as IntelliJ IDE Matrix
Dev->>GH: push / PR triggers workflows
GH->>Runner: start job (checkout, setup JDK 21 Zulu, gradle cache, chmod gradlew)
Runner->>Gradle: build project / dependencies (e.g. :stability-runtime)
alt IntelliJ Plugin workflow
GH->>IDE: matrix (2024.1,2024.2,2024.3) — build plugin for each IDE (continue-on-error)
IDE->>Gradle: build plugin artifacts
Gradle->>GH: upload artifacts/results
GH->>Dev: artifact links, status
else Module Tests workflow
par Parallel module jobs
Runner->>Gradle: run module-specific build/test tasks
Gradle->>GH: upload test reports on failure
end
Runner->>Gradle: integration-test publishes to Maven Local and verifies artifacts
Gradle->>GH: upload integration results
GH->>Dev: test reports, status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/intellij-plugin.yml (1)
1-148: Suggested refactor: Extract repeated job setup into a reusable workflow.All four jobs repeat the same setup steps (checkout, JDK 21, chmod, stability-runtime build). This violates DRY and increases maintenance burden if setup changes are needed.
Consider creating a
.github/workflows/setup-plugin-env.ymlreusable workflow that encapsulates the common setup, then call it from each job:# .github/workflows/setup-plugin-env.yml (new reusable workflow) on: workflow_call: jobs: setup: runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v4 - name: Set up JDK 21 uses: actions/setup-java@v4 with: distribution: 'zulu' java-version: 21 cache: 'gradle' - name: Grant execute permission for gradlew run: chmod +x gradlew - name: Build stability-runtime (dependency) run: ./gradlew :stability-runtime:build --no-daemon --stacktraceThen simplify each job to call this workflow. However, given the "Chill" review strictness, this refactor is optional and can be deferred.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/intellij-plugin.yml(1 hunks).github/workflows/module-tests.yml(1 hunks)gradle/libs.versions.toml(1 hunks)stability-compiler/build.gradle.kts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Lint Module Tests
- GitHub Check: Compiler Tests (FIR/IR Dumps)
- GitHub Check: Gradle Plugin Tests
- GitHub Check: Compiler Plugin Tests
- GitHub Check: Runtime Module Tests
- GitHub Check: Plugin Compatibility Check (2024.3)
- GitHub Check: Plugin Compatibility Check (2024.2)
- GitHub Check: Plugin Compatibility Check (2024.1)
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: Test IntelliJ Plugin
- GitHub Check: API check
- GitHub Check: Spotless check
- GitHub Check: Build and Tests
🔇 Additional comments (4)
stability-compiler/build.gradle.kts (2)
30-30: Questionable: Adding explicit kotlin-stdlib dependency.Adding
compileOnly(libs.kotlin.stdlib)appears redundant. The Kotlin stdlib is typically provided transitively by thekotlin-jvmplugin already applied (line 17) and should be available on the compile classpath without an explicit declaration. This may indicate either:
- The removal of the Compose compiler (which may have transively provided stdlib) necessitates an explicit declaration, or
- The dependency is unnecessary and adds maintenance overhead.
Verify the necessity of this dependency by checking if stability-compiler compiled and tested successfully before this change without an explicit stdlib declaration. If it did, this dependency can be removed to reduce redundancy.
29-43: Removal of kotlin-compile-testing may impact test capabilities.The removal of
testImplementation("com.github.tschuchortdev:kotlin-compile-testing:1.6.0")significantly reduces the module's ability to perform compile-time testing. This is a key library for testing Kotlin compiler plugins. Given that this module is the stability compiler plugin itself, ensure that:
- The tests in this module do not rely on kotlin-compile-testing for compilation validation
- Alternative testing mechanisms are in place (e.g., integration tests via the module-tests workflow)
- Test coverage is not reduced by this removal
Please verify that the existing tests in stability-compiler continue to function without kotlin-compile-testing and that test coverage for the compiler plugin remains adequate.
.github/workflows/module-tests.yml (1)
167-167: Verify integration-test job dependencies.The
integration-testjob (line 167) depends on[compiler-plugin-tests, runtime-tests, gradle-plugin-tests, lint-tests]but excludes thecompiler-testsjob. This appears inconsistent:
compiler-plugin-teststests the stability-compiler module (line 14)compiler-teststests the compiler-tests module (line 44) — a distinct module with FIR/IR dumpsClarify whether
compiler-testsshould be included in the integration-test dependencies, or if its exclusion is deliberate (e.g., it's optional or runs independently).Please verify the intended dependency graph. If compiler-tests should be part of the integration-test prerequisites, update line 167:
- needs: [compiler-plugin-tests, runtime-tests, gradle-plugin-tests, lint-tests] + needs: [compiler-plugin-tests, compiler-tests, runtime-tests, gradle-plugin-tests, lint-tests]gradle/libs.versions.toml (1)
30-30: Remove the concern about redundancy—compileOnly(libs.kotlin.stdlib)is the correct pattern for Kotlin compiler plugins.For Kotlin compiler plugins, the recommended approach is to use a compile-only dependency on the stdlib. The stability-compiler module is a compiler plugin (as indicated by its
kotlin-compiler.embeddabledependency), not a regular JVM library. The explicitcompileOnly(libs.kotlin.stdlib)declaration is intentional and necessary for proper classpath isolation and ensuring the correct version is used—it is not redundant despite thekotlin("jvm")plugin being present.Likely an incorrect or invalid review comment.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/module-tests.yml (1)
14-162: Consider extracting shared job setup into a composite action to reduce duplication.Each job repeats the same pattern:
actions/checkout@v4→actions/setup-java@v4(JDK 21, Zulu) →chmod +x gradlew→ build/test → conditional artifact upload. This duplication spans 140+ lines and makes maintenance harder if the setup steps need to change.Consider extracting this into a reusable composite action (e.g.,
.github/actions/setup-gradle-env/action.yml) to define the common setup once, then reuse it across jobs.Example composite action structure:
# .github/actions/setup-gradle-env/action.yml name: 'Setup Gradle Environment' runs: using: 'composite' steps: - uses: actions/checkout@v4 - uses: actions/setup-java@v4 with: distribution: 'zulu' java-version: 21 cache: 'gradle' - run: chmod +x gradlew shell: bashThen in each job:
steps: - uses: ./.github/actions/setup-gradle-env - name: Build stability-compiler run: ./gradlew :stability-compiler:build --no-daemon --stacktrace # ... rest of job stepsThis approach reduces duplication, improves maintainability, and aligns with the DRY principle.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/module-tests.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Runtime Module Tests
- GitHub Check: Build and Tests
- GitHub Check: Build IntelliJ Plugin
- GitHub Check: API check
- GitHub Check: Plugin Compatibility Check (2024.2)
- GitHub Check: Spotless check
- GitHub Check: Plugin Compatibility Check (2024.3)
- GitHub Check: Plugin Compatibility Check (2024.1)
- GitHub Check: Test IntelliJ Plugin
🔇 Additional comments (2)
.github/workflows/module-tests.yml (2)
1-162: The provided code appears truncated; the integration-test job is missing.Based on past review comments, the file should include an
integration-testjob (with a typo in the name at line 165: "Integration Testq" → "Integration Test") and Maven Local publication verification steps extending to line 189+. The provided annotated code ends at line 162 and is incomplete.Please provide the full file including the integration-test job and its verification steps so I can complete the review.
1-12: Workflow configuration is well-structured.The trigger configuration (push to main, PR to all branches) and concurrency settings with
cancel-in-progress: truefollow GitHub Actions best practices and will prevent redundant workflow runs.
Add IntelliJ plugin & module tests.
Summary by CodeRabbit