Repository navigation
Improve unit test coverage for controllers, webhooks and metrics (baseline @ a0f33cc) #3202
Description
What feature you would like to be added?
Improve unit test coverage for the controllers, webhooks and metrics, starting from a measured baseline, and publish coverage in CI so it's tracked per PR.
Baseline commit: a0f33cccc54858516d8954e7aef2c2bad0eecc76 (2026-09-14, "fix: stop adding SparkApplication owner reference to executor pods (#3152)"). Master will move; all line numbers and gaps in this issue refer to this commit.
Headline: total statement coverage is 34.1%. Several operational packages are at 0%: internal/metrics, pkg/tls, and the mutating/validating webhook configuration controllers. The SparkConnect reconcile loop is never exercised by tests.
Why is this needed?
Measured baseline
make unit-test at this commit (Go 1.25, envtest 1.35, run locally on macOS): all packages pass, 0 failures.
Total statement coverage: 34.1% (go tool cover -func=cover.out). This total includes generated code (pkg/client/**, zz_generated*) and cmd/ wiring, so it understates coverage of hand-written logic.
Per-package figures below are statement coverage as reported by go test (coverage: X% of statements).
| Package | Coverage | Src LOC | Test cases | Notes |
|---|---|---|---|---|
| pkg/features | 100.0% | 45 | 5 | |
| internal/scheduler/kubescheduler | 90.2% | 143 | 8 | |
| internal/scheduler/yunikorn | 86.9% | 130 | 2 | |
| internal/webhook | 82.6% | 1628 | 84 | Good depth |
| internal/controller/sparkapplication | 63.3% | 3828 | 97 | Some state-machine states have no scenario |
| pkg/certificate | 61.1% | 410 | 10 | Secret sync paths not directly tested |
| internal/scheduler/volcano | 58.3% | 200 | 1 | |
| internal/controller/sparkconnect | 52.3% | 812 | 23 | Helpers tested; Reconcile() never called in tests |
| pkg/util | 48.1% | 1001 | 122 | 30/81 funcs not named in tests |
| internal/controller/scheduledsparkapplication | 26.1% | 495 | 20 | One Reconcile() call in tests |
| internal/scheduler/yunikorn/resourceusage | 16.1% | 226 | 5 | Spark memory-overhead maths largely untested |
| api/v1beta2 | 3.6% | 530 | 9 | Figure includes generated deepcopy; hand-written defaults/helpers are small |
| internal/metrics | 0% | 563 | 0 | No test files |
| pkg/tls | 0% | 70 | 0 | No test files |
| internal/controller/mutatingwebhookconfiguration | 0% | 204 | 0 | No test files |
| internal/controller/validatingwebhookconfiguration | 0% | 204 | 0 | No test files |
| internal/scheduler, pkg/scheme, pkg/version, api/v1alpha1 | 0% | small | 0 | Interfaces, registration, types |
| cmd/operator/** | 0% | ~980 | 0 | Startup wiring; low priority |
Describe the solution you would like
Candidate gaps, by priority
P1: zero or low coverage on core behaviour
- SparkConnect reconcile loop (52.3%).
internal/controller/sparkconnect/reconciler.go:Reconcile()is never invoked by tests.createOrUpdateServerPod(L298),createOrUpdateServerService(L512),createOrUpdateConfigMap(L249),updateSparkConnectStatus(L607),updateExecutorStatus(L628) andlistExecutorPods(L645) are only reachable through it. The suite already starts envtest (suite_test.go), so this can be added without new test infrastructure. - SparkApplication states with no scenario (63.3%).
internal/controller/sparkapplication/controller_test.gocovers New/Submitted, Running, Completed, Failed, Succeeding, Failing, Terminated/TTL and Suspend/Resume. It has noContextfor:reconcileFailedSubmissionSparkApplication(controller.go L451)reconcilePendingRerunSparkApplication(L547)reconcileInvalidatingSparkApplication(L583)reconcileUnknownSparkApplication(L773)
- Metrics (0%).
internal/metrics/sparkapplication_metrics.goandsparkpod_metrics.gohave no tests. Counter increments and decrements (for example, running count on state transitions) and the latency and execution-time observations are unverified. These can be tested withprometheus/testutiland need no cluster. - TLS config (0%).
pkg/tls/tls.go:SetupTLS(L26),ParseTLSVersion(L55) andParseCipherSuites(L68) have no tests. They are pure functions, so table tests are cheap. - ScheduledSparkApplication (26.1%). The controller has only one
Reconcile()call in tests. Paths that need a scenario:shouldStartNextRun(L298),killLastRunIfNotFinished(L337),checkAndUpdatePastRuns(L351),bookkeepPastRuns(L433) and concurrency policies. - YuniKorn memory maths (resourceusage 16.1%).
internal/scheduler/yunikorn/resourceusage/memory.go: overhead factor, PySpark and off-heap memory, andDriverPodRequests/ExecutorPodRequestsare only exercised indirectly viaTestSchedule. Direct table tests would pin the Spark memory-overhead maths.
P2: partial coverage
- Webhook configuration controllers (0%).
internal/controller/mutatingwebhookconfigurationandvalidatingwebhookconfigurationhave no tests for CA-bundle injection (updateMutatingWebhookConfiguration, L87). - Certificate secret sync (61.1%).
pkg/certificate/certificate.go:syncInternalCertSecret(L77),SyncCertManagerSecret(L131) and the parse/update helpers have no direct test. - Volcano (58.3%).
internal/scheduler/volcano/scheduler.go:syncPodGroupInClientMode(L141) andsyncPodGroupInClusterMode(L158).
P3: tooling
- Publish coverage in CI.
make unit-testalready writescover.out(Makefile L204), but CI (.github/workflows/integration.yaml, "Run go unit tests") doesn't surface it. Proposal: printgo tool cover -func=cover.out | tail -1in the job summary and uploadcover.htmlas an artifact, excludingpkg/client/**andzz_generated*. This is vendor-neutral and needs no external coverage service. - Makefile / envtest coupling. In
unit-test(Makefile L203) theKUBEBUILDER_ASSETS=...assignment runs on its own recipe line, so it has no effect on thego testline that follows. The suites work only because eachsuite_test.gohardcodesBinaryAssetsDirectorytobin/k8s/1.35.0-<os>-<arch>, whileENVTEST_K8S_VERSIONis derived from thek8s.io/apiversion. A future k8s bump may therefore break the envtest suites until those paths are updated. Proposal: export the variable on thego testline, or readKUBEBUILDER_ASSETSin the suites.
Proposed breakdown (sub-issues)
- CI: publish unit-test coverage summary and artifact, so the 34.1% baseline is tracked per PR (item 10)
- Makefile/envtest: fix
KUBEBUILDER_ASSETSand remove hardcoded 1.35.0 (item 11) - SparkConnect: envtest reconcile tests (item 1)
- SparkApplication: missing state-machine scenarios (item 2)
- Metrics: unit tests with
prometheus/testutil(item 3) - pkg/tls: table tests (item 4)
- ScheduledSparkApplication: schedule and concurrency-policy scenarios (item 5)
- Webhook configuration controllers and certificate sync (items 7 and 8)
- Scheduler memory maths: YuniKorn and Volcano (items 6 and 9)
Describe alternatives you have considered
- External coverage service (Codecov/Coveralls): not proposed. It adds a third-party dependency and token.
go tool coveroutput in the job summary plus an HTML artifact is vendor-neutral and sufficient. - Hard coverage gate in CI now: not proposed. A threshold should be agreed after the baseline has been published for a while, to avoid blocking unrelated PRs.
- Relying on e2e tests for coverage: e2e (
test/e2e) needs a kind cluster and is slower. The gaps listed here are testable with plain unit tests or the existing envtest suites.
Additional context
Method:
- Measured statement coverage from
make unit-testat this commit. - Static analysis of the test surface to locate specific gaps (test cases per package, and functions that no
_test.goreferences). A function with no direct test may still be covered throughReconcile()or another public entry point.
Out of scope
- Generated code (
pkg/client/**,zz_generated.deepcopy.go) - e2e tests (
test/e2e) and drift tests (test/drift) - A hard coverage gate in CI; that is to be discussed once the baseline is published
Love this feature?
Give it a 👍 We prioritize the features with most 👍
Activity
vikas-saxena02 commentedon Sep 27, 2026
🌱 Note for new contributors
Thanks for your interest! Many of these tasks are well-scoped unit tests, so this is a good place to start contributing.
Please don't assign this issue (#3202) to yourself. It's the parent tracking issue. Once maintainers have agreed on the breakdown, I'll create sub-issues and link them here, and those are the ones to pick up. Comment
/assignon the sub-issue you'd like to take.Thank you! 🙏
UchihaIthachi commentedon Oct 17, 2026
Hi @vikas-saxena02, I'd like to take the metrics part (item 3).
internal/metrics still has no tests on master and I didn't find a PR for it.
I'd cover both files with table tests in the same style as pkg/tls/tls_test.go:
the running/success/failure counts through create, update and delete
(including Pending -> Succeeded and Running -> Unknown), the label fallbacks
in getMetricLabels, and the submit/start latency histograms in
sparkapplication_metrics.go. No envtest, and Register() isn't called so the
global registry stays untouched.
I can send it as one PR, or as two (executor metrics first, then application
metrics) if smaller PRs are easier to review. Whichever you prefer.
One small thing: incFailureCount logs "Increased Spark executor running count".
I can fix that separately if you want.
Could you open a sub-issue for this?
vikas-saxena02 commentedon Oct 18, 2026
@UchihaIthachi plz go for it... but please write the test cases in ginkgo. see #3231 and #3232 for reference. i dont have privilegese to create subtasks for features on the repo