Skip to content

fix: getAppSetStatus incorrectly treats ApplicationSetUpToDate=True as Error - #284

Merged
keithchong merged 1 commit into
redhat-developer:mainfrom
aali309:GITOPS-11094
Sep 1, 2026
Merged

fix: getAppSetStatus incorrectly treats ApplicationSetUpToDate=True as Error#284
keithchong merged 1 commit into
redhat-developer:mainfrom
aali309:GITOPS-11094

Conversation

@aali309

@aali309 aali309 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

…s Error

Signed-off-by: Atif Ali <atali@redhat.com>
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: ee7197e5-980a-4365-9f94-14e573e5b45b

📥 Commits

Reviewing files that changed from the base of the PR and between 2da6a8f and ebab759.

📒 Files selected for processing (2)
  • src/gitops/utils/gitops.test.ts
  • src/gitops/utils/gitops.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved ApplicationSet health reporting to correctly identify outdated resources as an error.
    • Healthy ApplicationSets now remain marked healthy when unrelated condition types are present.
    • Added coverage for healthy, outdated, and mixed-condition status scenarios to improve reliability.

Walkthrough

The ApplicationSet status evaluation now checks ResourcesUpToDate correctly. Tests cover healthy conditions, resource errors, and unrelated condition types.

Changes

ApplicationSet status evaluation

Layer / File(s) Summary
Condition evaluation and test coverage
src/gitops/utils/gitops.ts, src/gitops/utils/gitops.test.ts
getAppSetStatus returns Error when ResourcesUpToDate is not True. Tests cover healthy conditions, ResourcesUpToDate=False, and ApplicationSetUpToDate as a non-condition type.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ebab7

This localized status-handling change has no actionable merge-blocking risk remaining and is merge-ready after normal checks and review.

Suggested reviewers: keithchong, wtam2018

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: preventing ApplicationSetStatus from treating ApplicationSetUpToDate=True as an error.
Description check ✅ Passed The description links to GITOPS-11094, which matches the stated objective and changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI

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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 12.40%. Comparing base (3bd1dab) to head (ebab759).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
+ Coverage   11.30%   12.40%   +1.10%     
==========================================
  Files         169      171       +2     
  Lines        6679     6835     +156     
  Branches     2235     2319      +84     
==========================================
+ Hits          755      848      +93     
- Misses       5923     5986      +63     
  Partials        1        1              
Flag Coverage Δ
unit-tests 12.40% <100.00%> (+1.10%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trdoyle81

Copy link
Copy Markdown
Member

/lgtm
/approve

@keithchong keithchong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@keithchong
keithchong merged commit fd5d3d8 into redhat-developer:main Sep 1, 2026
5 checks passed
@aali309
aali309 deleted the GITOPS-11094 branch September 1, 2026 14:54
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.

4 participants