Fix flaky test: Security::ScanResultPolicies::PolicyViolationDetails#fail_closed_policies
Summary
This MR fixes a flaky test that was failing due to non-deterministic database query results when multiple approval rules exist for the same policy.
Test Details:
- File:
ee/spec/lib/security/scan_result_policies/policy_violation_details_spec.rb - Lines: 292, 352
- Issue: https://gitlab.com/gitlab-org/quality/test-failure-issues/-/issues/13123
Root Cause
The test creates violations for policy3 without explicitly associating an approval_policy_rule:
create(:scan_result_policy_violation, project: project, merge_request: merge_request,
scan_result_policy_read: policy3) # ← NO approval_policy_rule set
create(:report_approver_rule, :scan_finding, merge_request: merge_request,
scan_result_policy_read: policy3, name: 'Other')
create(:report_approver_rule, :scan_finding, merge_request: merge_request,
scan_result_policy_read: policy3, name: 'Other 2')
The violations method in the implementation relies on scan_result_policy_rules, which queries:
merge_request
.approval_rules
.with_scan_result_policy_read
.applicable_to_branch(merge_request.target_branch)
.index_by(&:scan_result_policy_id) # ← Returns ONLY ONE rule per policy_id
When multiple approval rules exist for the same scan_result_policy_id, index_by returns only ONE rule non-deterministically. This causes the test to:
- Sometimes find the "Other" rule → test passes
- Sometimes find the "Other 2" rule → test passes
- Sometimes find neither → test fails with missing "Other"
The Fix
Explicitly associate the approval_policy_rule when creating the violation for policy3, ensuring deterministic behavior:
let(:policy3_rule) { create(:approval_policy_rule) }
before do
create(:scan_result_policy_violation, project: project, merge_request: merge_request,
scan_result_policy_read: policy3, approval_policy_rule: policy3_rule) # ← Explicit association
create(:report_approver_rule, :scan_finding, merge_request: merge_request,
scan_result_policy_read: policy3, approval_policy_rule: policy3_rule, name: 'Other')
create(:report_approver_rule, :scan_finding, merge_request: merge_request,
scan_result_policy_read: policy3, approval_policy_rule: policy3_rule, name: 'Other 2')
...
end
This matches the pattern used for policy_warn_mode on line 290, which already had approval_policy_rule: warn_mode_policy_rule set.
Verification
The fix ensures that:
- The violation for
policy3has a direct association topolicy3_rule - Both approval rules "Other" and "Other 2" are associated with the same
policy3_rule - The
violationsmethod will consistently find the correct rule via the direct association - The test will deterministically return
["Other", "Policy", "Warn mode"]
Confidence Score: 95%
The fix directly addresses the race condition by eliminating the non-deterministic database query. The only remaining uncertainty is whether there are other similar patterns in the codebase that might need the same fix.