villebro opened a new pull request, #109: URL: https://github.com/apache/superset-kubernetes-operator/pull/109
## Summary A review of the test suite found two kinds of fragmentation that the project's testing philosophy discourages: clusters of single-assertion unit tests that each repeat a near-identical setup, and e2e specs that each stand up their own full Superset just to check one facet. This PR consolidates both — collapsing pure-function unit tests into table-driven/subtest groups, and folding several narrow e2e scenarios into one comprehensive deployment-lifecycle journey that amortizes the expensive cluster setup. It also documents why CEL/schema tests must stay at the integration tier so the tradeoff isn't re-litigated. No production code changes — this is a test-readability, maintainability, and CI-cost refactor with no coverage loss. ## Details ### Unit tests (`internal/controller/`) Collapsed pure-function test families into parent tests with named `t.Run` subtests (−43 top-level functions, same assertions). Each parent test carries a doc comment describing the general behavior under test, and subtests keep | File | Before → After | Parent tests | |---|---|---| | `deployment_builder_test.go` | 14 → 4 | `TestBuildDeploymentSpec`, `TestBuildServiceSpec` | | `clone_test.go` | 51 → 40 | `TestBuildCloneScript`, `TestBuildCloneCommand`, `TestResolveCloneImage` | | `lifecycle_create_db_test.go` | 17 → 7 | `TestBuildCreateDatabaseInitContainer`, `TestBuildStandardTaskFlatSpec` | | `maintenance_test.go` | 18 → 9 | `TestRenderMaintenanceHTML`, `TestMaintenanceNginxConf`, `TestBuildMaintenanceFlatSpec`, `TestResolveWebServerPort` | | `status_test.go` | 14 → 11 | `TestSetCondition` | Left untouched by design: reconcile-based families, the clone checksum/pipeline edge-case tests, the already table-driven `internal/config` and `internal/resolution` packages, and the exemplary `lifecycle_*` files. ### E2E tests (`test/e2e/`) Replaced five separate per-scenario CR deploys with **one `Ordered` deployment journey** (`superset_journey_test.go`). It deploys a single comprehensive CR (all components + referenced secret + per-component config/replica overrides + autoscaling/PDB + networking + monitoring + NetworkPolicy + supervised upgrade) in `BeforeAll`, then asserts each facet as an independently-named checkpoint `It`: components → services → rendered config → HPA/PDB → NetworkPolicy → ServiceMonitor → Ingress→Gateway switch → supervised image upgrade → delete + owner-ref GC with referenced Secret preserved. - Removed `superset_resources_test.go` and `superset_deletion_test.go` (folded). - Removed the two CR-reconcile specs from `e2e_test.go` and the image-change spec from `superset_lifecycle_test.go` (folded). - Kept focused/distinct: failed-migration recovery, secret-key rotation, CEL validation smoke, and the operator-infra (controller-up/metrics) specs. This drops the suite from 5 full CR deploys to 1 shared deploy, which is where e2e CI time actually goes (per-scenario cluster work, not fixed suite startup). ### Docs Added a note to the Test pyramid section of `development-guidelines.md`: the controller-runtime fake client does not enforce CRD OpenAPI schema or CEL, so schema/CEL tests must live at the integration tier and can't be moved down to fake-client unit tests. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
