Repository navigation
Expect both mode rules in no-results prefetch test - #1949
Conversation
The no-taskrun-results test was written before conforma#1871 added mode_not_permissive_v2, which also fires for the "permissive" mode, so its expected results were missing the v2 entry. Co-Authored-By: glm-5-3 <noreply@z.ai>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Update denial expectations policy/release/prefetch_dependencies/prefetch_dependencies_test.rego |
The test expects the legacy and v2 denial results. The legacy result no longer includes an effective_on value. |
Priority:
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Bug fix
Suggested reviewers: cuipinghuo, dheerajodha
Merge Risk: 
1b908
-
The test expectations now cover both permissive-mode denial results, with no merge-blocking risk identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly describes the main test change: expecting both mode rules in the no-results prefetch test. Description check The description explains what changed, why the change was needed, and reports test results. It does not include the template headings or a Jira ticket link, but the required context is otherwise compl… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains what changed, why the change was needed, and reports test results. It does not include the template headings or a Jira ticket link, but the required context is otherwise complete.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
Comment @coderabbitai help to get the list of available commands.
|
<!-- fullsend:risk-assessment --> DetailsSingle test file fix (16 lines, XS) correcting a missing expected result entry caused by two PRs landing out of order; no protected paths, no security or dependency changes, and the scope precisely matches the described regression. |
|
Looks good to me <!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJmaW5kaW5ncyI6W119 --> |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
ready-for-merge
Retro: PR #1949 — Expect both mode rules in no-results prefetch testThis was a clean, well-handled workflow. PR #1949 was a human-authored XS fix (1 file, +11/−5) correcting test expectations in Judge scores
Notes on existing issues
|
PR #1871 added mode_not_permissive_v2, which fires alongside the legacy mode_not_permissive rule when the mode is "permissive". The no-taskrun-results test from #1945 was written before that landed, so its expected results were missing the v2 entry and the test failed on main after both changes merged.
The expected results now include both rules, matching test_mode_permissive_violation. Full suite passes locally: 1221/1221.
Co-Authored-By: glm-5-3 <noreply@z.ai>