test: add Kotlin verification smoke test - #3513
Conversation
Adds a Maven-compiled Kotlin test source set to operator-framework-core and a smoke test verifying that a checked (non-RuntimeException) Exception thrown from a Kotlin DependentResource is properly caught and reported by the workflow executor, so that retries are triggered as expected. Kotlin does not have checked exceptions, so Kotlin code can throw a checked Exception from an overridden method without declaring it, even though the Java DependentResource#reconcile signature does not declare `throws Exception`. Before operator-framework#2965 this exception would not have been caught by NodeExecutor, since it only handled RuntimeException, silently swallowing the error and preventing retries. Closes operator-framework#2967
There was a problem hiding this comment.
Pull request overview
Adds Kotlin-based test coverage to ensure the workflow executor correctly captures non-RuntimeException failures thrown from Kotlin-implemented DependentResources, preventing silent bypass of retry/error aggregation logic.
Changes:
- Introduces a Kotlin smoke test that throws a checked
ExceptionfromDependentResource.reconcileand asserts it is surfaced viaerroredDependentsand aggregated exceptions. - Updates
operator-framework-corebuild to compilesrc/test/kotlinviakotlin-maven-pluginand adds Kotlin stdlib as a test-scoped dependency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| operator-framework-core/src/test/kotlin/io/javaoperatorsdk/operator/processing/dependent/workflow/KotlinCheckedExceptionDependentResourceTest.kt | Adds Kotlin smoke test validating workflow error capture/aggregation for checked exceptions thrown from Kotlin dependents |
| operator-framework-core/pom.xml | Adds Kotlin test compilation support and Kotlin stdlib dependency (test scope) to enable the new Kotlin test |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@hej090224 thank you for the PR, there is a syntaxt error. But I was wondering if this is enough to validate this. Would be better IMO to add a sample, a minimalist operator kontlin, and the related E2E test as for other samples, that tests the whole lifecycle. Would you mind to implement this issue that way? The controller could just apply a config map, copy a String value from custom resource spec. But that would show also that deserialization works from fabric8 client, and if the whole runtime is working. |
Closes #2967
What this does
Adds a Kotlin-based smoke test to
operator-framework-coreverifying that JOSDK behaves correctly when aDependentResourceis implemented in Kotlin.Kotlin has no concept of checked vs. unchecked exceptions, so Kotlin code can throw a plain
Exceptionfrom an overridden method (e.g.DependentResource#reconcile) without declaring it — even though the Java-declared method signature doesn't includethrows Exception. Before #2965,NodeExecutoronly caughtRuntimeException, so such an exception thrown from Kotlin would silently bypass the workflow's error handling and not trigger a retry.The new test (
KotlinCheckedExceptionDependentResourceTest.kt):DependentResourcewhosereconcilethrows a custom checkedException.Workflow/WorkflowReconcileExecutor/NodeExecutor(no mocked cluster needed).WorkflowReconcileResult#getErroredDependents()and thatthrowAggregateExceptionIfErrorsPresent()throwsAggregatedOperatorException, confirming a retry would be triggered.I verified this test actually exercises the fix: temporarily reverting
NodeExecutor'scatch (Exception e)back tocatch (RuntimeException e)(the pre-#2965 behavior) makes the test fail, since the checked exception is then silently swallowed andgetErroredDependents()stays empty.Build changes
kotlin-maven-plugin(test scope only,2.4.10) tooperator-framework-core/pom.xml, compiling sources undersrc/test/kotlin. No other module is affected, and no Kotlin dependency is added to the main/runtime classpath.Relation to #2965
#2965 fixed
NodeExecutor,AbstractWorkflowExecutor, andPollingEventSourceto catchExceptioninstead ofRuntimeException(with// Exception is required because of Kotlincomments already in place), and opened #2967 as a follow-up to add tests validating this behavior with actual Kotlin code. I also grepped the codebase for any remainingcatch (RuntimeExceptionin main sources and found none left to fix.I scoped this PR to a unit-test-level smoke test (no real Kubernetes cluster required) so it's fast and fully verifiable in CI; a full Kotlin-based
*IT.java-style integration test againstLocallyRunOperatorExtensioncould be a good follow-up if maintainers want broader coverage.Test plan
./mvnw -pl operator-framework-core -am test— 646 tests passcatch (RuntimeException)behavior, and passes against currentcatch (Exception)behavior