Skip to content

KFLUXINFRA-4426 render-diff: skip empty-base overlays - #13840

Draft
oswcab wants to merge 1 commit into
mainfrom
KFLUXINFRA-4426/render-diff-empty-base
Draft

KFLUXINFRA-4426 render-diff: skip empty-base overlays#13840
oswcab wants to merge 1 commit into
mainfrom
KFLUXINFRA-4426/render-diff-empty-base

Conversation

@oswcab

@oswcab oswcab commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

instead of reporting BUILD ERROR

Components that use empty-base overlays (resources: []) to provide a valid no-op ArgoCD Application for non-targeted clusters produce no kustomize output. render-diff was treating this as "does not exist on either ref" and reporting a BUILD ERROR, which creates noise in PR reviews.

Now when a directory exists on at least one ref but kustomize produces no output, the component is silently skipped instead of reported as an error.

@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the approved label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Kustomize Render Diff

Comparing 66d24b333367b8a941

No render differences detected.

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:39 PM UTC · Completed 6:52 PM UTC

Commit: e9d02ea · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.16

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.49%. Comparing base (66d24b3) to head (481a1eb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #13840      +/-   ##
==========================================
+ Coverage   60.38%   60.49%   +0.10%     
==========================================
  Files          24       24              
  Lines        1628     1630       +2     
==========================================
+ Hits          983      986       +3     
  Misses        563      563              
+ Partials       82       81       -1     
Flag Coverage Δ
go 60.49% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fullsend-ai-review fullsend-ai-review Bot added the risk/low PR risk: low label Sep 2, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

Risk Assessment: low (1/5)

Details

Small Go tooling change (2 files, 31 lines) with 50% test coverage ratio in stable code untouched for 175 days, no protected or security-sensitive paths, minimal risk.

Previous run

Risk Assessment: low (1/5)

Details

Small Go tooling change (2 files, 31 lines) with 50% test coverage ratio in stable code untouched for 175 days, no protected or security-sensitive paths, minimal risk.

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [test adequacy] infra-tools/internal/renderdiff/engine_test.go — The new test TestEngine_EmptyBase_NewComponent covers the case where the directory exists on HEAD but not on base and produces nil output. However, there is no corresponding test for the symmetric case where the directory exists on base but not on HEAD (removed component that had empty output), nor for the case where the directory exists on both refs but both produce nil output. These are valid scenarios exercised by the new DirExists guard in buildPair.
    Remediation: Consider adding tests for the symmetric cases: (1) base exists with nil output, head does not exist; (2) both exist with nil output.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Looks good to me

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 2, 2026
…rting BUILD ERROR

Components that use empty-base overlays (resources: []) to provide a
valid no-op ArgoCD Application for non-targeted clusters produce no
kustomize output. render-diff was treating this as "does not exist on
either ref" and reporting a BUILD ERROR, which creates noise in PR
reviews.

Now when a directory exists on at least one ref but kustomize produces
no output, the component is silently skipped instead of reported as an
error.

Assisted-by: Claude Code
@oswcab
oswcab force-pushed the KFLUXINFRA-4426/render-diff-empty-base branch from 029df5d to 481a1eb Compare September 2, 2026 19:37
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:37 PM UTC · Completed 7:53 PM UTC

Commit: 66d24b3 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.96

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Sep 2, 2026
@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oswcab, sadlerap

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants