avirajkhare00 commented on PR #3019: URL: https://github.com/apache/iggy/pull/3019#issuecomment-4115338689
I dug through the hardcoded bits in this PR. The main problem is not the number of scenarios, it is that `_test.yml` is now carrying some chart-owned values that already exist under `helm/charts/iggy/`. There are two buckets here: 1. Reasonable CI-owned hardcoding - pinned Helm/kind versions and checksums - kind cluster config - ingress-nginx install - smoke-test hosts / ingress class / namespace / release names Those are properties of the CI environment and smoke path, so keeping them in the workflow is fine. 2. Chart-owned values that should ideally be reused from `helm/charts/iggy/` - chart `version` / `appVersion` from `helm/charts/iggy/Chart.yaml` - default image tags from `helm/charts/iggy/values.yaml` - render scenario inputs that are really chart fixtures rather than CI wiring That second bucket is where the PR feels too hardcoded, because it creates a second source of truth in CI. If you want to keep this structure, I would suggest a cleaner split: - leave CI/runtime setup in `_test.yml` - move chart-specific test fixtures into the `helm/charts/iggy/` tree (for example dedicated values files under a small `ci/` or `tests/` subdir), or derive the asserted values from chart files directly That keeps the Helm chart as the owner of chart data and makes version/tag bumps less noisy. -- 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]
