chore: enhance desiredrelease postgres tests#891
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 35 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughEnabled DB-backed integration tests in CI, removed the Postgres-backed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/workspace-engine/svc/controllers/desiredrelease/variableresolver/getters_postgres_test.go (1)
140-166: Make the workspace-filter subtest self-contained to avoid vacuous pass.At Line 160-Line 165, this can pass even if no same-workspace rules are returned. Consider inserting one same-workspace rule in this subtest and asserting it is returned while the other-workspace rule is excluded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/desiredrelease/variableresolver/getters_postgres_test.go` around lines 140 - 166, The subtest "does not return rules from other workspaces" is vacuous because it only inserts an other-workspace rule; update the test to insert an additional relationship_rule row tied to f.workspaceID (e.g., a rule with reference "same.ref") before calling getter.GetRelationshipRules(ctx, f.workspaceID), then assert that the returned rules include the "same.ref" rule and do not include "other.ref"; ensure you add matching cleanup for the inserted same-workspace rule and keep using getter.GetRelationshipRules and the existing pool.Exec/require.NoError pattern so the assertions meaningfully verify workspace filtering.apps/workspace-engine/svc/controllers/desiredrelease/getters_postgres_test.go (1)
489-509: Avoid cross-subtest data coupling in aggregation assertions.Both subtests depend on rows inserted by earlier subtests in the same parent test. Please make each subtest create its own prerequisites so it remains reliable when run independently.
Also applies to: 568-580
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/desiredrelease/getters_postgres_test.go` around lines 489 - 509, The subtests are relying on rows inserted by other subtests, causing cross-test coupling; inside each subtest (e.g., the "null environment and resource matches any target" case) insert its own prerequisite rows instead of depending on earlier inserts: create distinct policy_skip rows for the targeted skip and the global skip via pool.Exec with fresh UUIDs, call getter.GetPolicySkips(versionID.String(), f.environmentID.String(), f.resourceID.String()) and assert the exact expected count (or validate by ID) rather than using a >= that assumes prior state; apply the same isolation change to the other related subtest that asserts skips (the block around the later assertion) so each subtest is self-contained and deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/workspace-engine/svc/controllers/desiredrelease/getters_postgres_test.go`:
- Around line 489-509: The subtests are relying on rows inserted by other
subtests, causing cross-test coupling; inside each subtest (e.g., the "null
environment and resource matches any target" case) insert its own prerequisite
rows instead of depending on earlier inserts: create distinct policy_skip rows
for the targeted skip and the global skip via pool.Exec with fresh UUIDs, call
getter.GetPolicySkips(versionID.String(), f.environmentID.String(),
f.resourceID.String()) and assert the exact expected count (or validate by ID)
rather than using a >= that assumes prior state; apply the same isolation change
to the other related subtest that asserts skips (the block around the later
assertion) so each subtest is self-contained and deterministic.
In
`@apps/workspace-engine/svc/controllers/desiredrelease/variableresolver/getters_postgres_test.go`:
- Around line 140-166: The subtest "does not return rules from other workspaces"
is vacuous because it only inserts an other-workspace rule; update the test to
insert an additional relationship_rule row tied to f.workspaceID (e.g., a rule
with reference "same.ref") before calling getter.GetRelationshipRules(ctx,
f.workspaceID), then assert that the returned rules include the "same.ref" rule
and do not include "other.ref"; ensure you add matching cleanup for the inserted
same-workspace rule and keep using getter.GetRelationshipRules and the existing
pool.Exec/require.NoError pattern so the assertions meaningfully verify
workspace filtering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65f56fe6-0d31-4f16-bbab-deb3f2cdac9a
📒 Files selected for processing (4)
.github/workflows/apps-workspace-engine.yamlapps/workspace-engine/svc/controllers/desiredrelease/getters_postgres.goapps/workspace-engine/svc/controllers/desiredrelease/getters_postgres_test.goapps/workspace-engine/svc/controllers/desiredrelease/variableresolver/getters_postgres_test.go
💤 Files with no reviewable changes (1)
- apps/workspace-engine/svc/controllers/desiredrelease/getters_postgres.go
Summary by CodeRabbit
Tests
Chores