fix: merge VMAnomaly child queries - #2509
Conversation
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/controller/operator/factory/vmanomaly/config/config_test.go">
<violation number="1" location="internal/controller/operator/factory/vmanomaly/config/config_test.go:2151">
P3: The new test's `assert.Contains(..., "default-child-query")` is a weak assertion: the model `main` in configRawYaml already references `queries: [default-child-query]`, so that string appears in `models.main.queries` even if the child query was never merged into `reader.queries`. The only thing actually guarding the child-config merge is the implicit `require.NoError` from `pos.Load` (validation fails when the referenced query is absent from reader.queries). Assert directly on the merged reader queries, e.g. check that the output contains `reader:` with `default-child-query` mapped to `expr: up`, so the test documents the intended behavior instead of relying on an incidental model reference.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| require.NoError(t, err) | ||
| loaded, err := pos.Load(cr, ac) | ||
| require.NoError(t, err) | ||
| assert.Contains(t, string(loaded), "default-child-query") |
There was a problem hiding this comment.
P3: The new test's assert.Contains(..., "default-child-query") is a weak assertion: the model main in configRawYaml already references queries: [default-child-query], so that string appears in models.main.queries even if the child query was never merged into reader.queries. The only thing actually guarding the child-config merge is the implicit require.NoError from pos.Load (validation fails when the referenced query is absent from reader.queries). Assert directly on the merged reader queries, e.g. check that the output contains reader: with default-child-query mapped to expr: up, so the test documents the intended behavior instead of relying on an incidental model reference.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/controller/operator/factory/vmanomaly/config/config_test.go, line 2151:
<comment>The new test's `assert.Contains(..., "default-child-query")` is a weak assertion: the model `main` in configRawYaml already references `queries: [default-child-query]`, so that string appears in `models.main.queries` even if the child query was never merged into `reader.queries`. The only thing actually guarding the child-config merge is the implicit `require.NoError` from `pos.Load` (validation fails when the referenced query is absent from reader.queries). Assert directly on the merged reader queries, e.g. check that the output contains `reader:` with `default-child-query` mapped to `expr: up`, so the test documents the intended behavior instead of relying on an incidental model reference.</comment>
<file context>
@@ -2107,3 +2107,46 @@ server:
+ require.NoError(t, err)
+ loaded, err := pos.Load(cr, ac)
+ require.NoError(t, err)
+ assert.Contains(t, string(loaded), "default-child-query")
+}
</file context>
Remove the VMAnomaly limitation to require reader.query, so that they can be provided in child
VMAnomalyConfig