Skip to content

Check for TEST_OUTPUT in RPM build tasks#1709

Open
jorris wants to merge 1 commit intoconforma:mainfrom
jorris:ROK-1119
Open

Check for TEST_OUTPUT in RPM build tasks#1709
jorris wants to merge 1 commit intoconforma:mainfrom
jorris:ROK-1119

Conversation

@jorris
Copy link
Copy Markdown
Contributor

@jorris jorris commented Apr 6, 2026

As with container builds, we want to verify that there's at least one
test run. This is a temporary duplicate set to warn, which will be removed
when we verify this change will not break the rpm build pipleline using the original
deny test.

JIRA: ROK-1119

As with container builds, we want to verify that there's at least one
SAST test run

JIRA: ROK-1119
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 6, 2026

📝 Walkthrough

Walkthrough

Two Rego policy files were modified to add RPM-specific test data validation. A new warning rule was added to test.rego that triggers when RPM attestations lack test data, alongside a corresponding test case in test_test.rego to verify the rule's behavior.

Changes

Cohort / File(s) Summary
Policy Rule Addition
policy/release/test/test.rego
Added a new warn rule that checks for empty lib.results_from_tests when PipelineRun attestations are present. Returns a warning result with metadata identifying it as RPM-specific test data validation, depends on attestation_type.known_attestation_type.
Policy Test Addition
policy/release/test/test_test.rego
Added test_rpm_needs_non_empty_data test rule that verifies the new warning rule emits a test.warn result with code test.rpm_test_data_found and message "No RPM test data found" when attestation task results omit TEST_OUTPUT.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding a check for TEST_OUTPUT in RPM build tasks, which is the core objective of this pull request.
Description check ✅ Passed The description is related to the changeset and explains the intent: verifying test runs in RPM builds by checking for TEST_OUTPUT, with context about it being a temporary duplicate warning.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
policy/release/test/test.rego 100.00% <100.00%> (ø)
policy/release/test/test_test.rego 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@policy/release/test/test.rego`:
- Around line 106-110: The warning currently uses lib.results_from_tests over
lib.pipelinerun_attestations and thus examines all TEST_OUTPUT entries; restrict
the scope to RPM-only before counting: filter lib.pipelinerun_attestations to
RPM attestations (e.g., by an attestation field that identifies RPM builds or
task name) and then derive results from that filtered set (or filter
lib.results_from_tests output for entries with TEST_OUTPUT that are tied to RPM
attestations) so that the check in warn contains result if uses only RPM-related
results when evaluating count(results) == 0.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c7eb1aa-5dc7-4979-a89a-5ff0f00638d2

📥 Commits

Reviewing files that changed from the base of the PR and between f9daefc and 6b3b8cd.

📒 Files selected for processing (2)
  • policy/release/test/test.rego
  • policy/release/test/test_test.rego

Comment on lines +106 to +110
warn contains result if {
count(lib.pipelinerun_attestations) > 0 # make sure we're looking at a PipelineRun attestation
results := lib.results_from_tests
count(results) == 0 # there are none at all

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Scope this warning to RPM attestations only

Line 108 uses lib.results_from_tests, which aggregates TEST_OUTPUT from all PipelineRun tasks, not RPM-only. That means this rule can miss missing RPM test output when non-RPM tasks contain TEST_OUTPUT, and can also warn on non-RPM-only inputs. Please filter attestations/results to RPM builds before evaluating count(results) == 0.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@policy/release/test/test.rego` around lines 106 - 110, The warning currently
uses lib.results_from_tests over lib.pipelinerun_attestations and thus examines
all TEST_OUTPUT entries; restrict the scope to RPM-only before counting: filter
lib.pipelinerun_attestations to RPM attestations (e.g., by an attestation field
that identifies RPM builds or task name) and then derive results from that
filtered set (or filter lib.results_from_tests output for entries with
TEST_OUTPUT that are tied to RPM attestations) so that the check in warn
contains result if uses only RPM-related results when evaluating count(results)
== 0.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine, because we're expecting one build pipeline attestation, or if there is multiple for some reason, all of them will be rpm builds.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simonbaird, that makes sense — since the policy is already scoped to the redhat_rpms collection and the pipeline will only ever have RPM build attestations in that context, the existing check is sufficient. I'll drop this concern.


📝 Learnings were identified and not saved (knowledge base disabled). Enable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants