Skip to content

fix(cli): gc 'print'/'tag' progress no longer shows NaN% or misleading '0 assets' - #1873

Open
lemon0333 wants to merge 1 commit into
aws:mainfrom
lemon0333:fix/gc-print-progress-messaging
Open

fix(cli): gc 'print'/'tag' progress no longer shows NaN% or misleading '0 assets'#1873
lemon0333 wants to merge 1 commit into
aws:mainfrom
lemon0333:fix/gc-print-progress-messaging

Conversation

@lemon0333

Copy link
Copy Markdown
Contributor

Fixes #625

Problem

Running cdk gc --action print (and --action tag on a bucket/repo that was already tagged) produced output that made users believe garbage collection was broken, even though it was working:

[NaN%] 0 files scanned: 0 assets (0.00 MiB) tagged, 0 assets (0.00 MiB) deleted.
[100.00%] 13 files scanned: 0 assets (0.00 MiB) tagged, 0 assets (0.00 MiB) deleted.
[NaN%] 0 files scanned: 0 assets (0.00 MiB) tagged, 0 assets (0.00 MiB) deleted.

Two things are wrong:

  1. [NaN%]ProgressPrinter.print() computes assetsScanned / totalAssets. For an empty bucket/repo (or the final flush of one) totalAssets is 0, so 0 / 0 renders as NaN%.
  2. "0 assets tagged, 0 assets deleted" — the print action intentionally never tags or deletes, so those counters are always 0. The message therefore reads as "nothing to do" even when stale/isolated assets exist and would be collected.

Fix

  • Guard the percentage math so an environment with nothing to scan reports [100.00%] instead of [NaN%].
  • For the print action, report the assets that are eligible for garbage collection (isolated assets not referenced by any deployed stack) — e.g. N assets (X MiB) eligible for deletion — instead of the misleading 0 tagged, 0 deleted line. The tag / delete-tagged / full output is unchanged.

This is a messaging/counter clarity fix only; garbage-collection semantics are unchanged.

Added unit tests under the gc test suite covering the NaN guard, a known-total percentage, and the print-action eligible-asset messaging.

Follow-up (not in this PR): a repeated tag run reports 0 tagged because the assets are already tagged; clarifying that "already tagged" state is a separate, larger change and left for a follow-up.

Checklist

  • This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed
    • Release notes for the new version:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

The garbage collection progress printer divided `assetsScanned / totalAssets`
without guarding against `totalAssets === 0`. Empty buckets/repos (and the final
flush of one) produced `[NaN%]`, and because the `print` action never tags or
deletes anything, its output always read `0 assets tagged, 0 assets deleted` even
when stale assets existed. Together these made users think `cdk gc` was broken
when it was working correctly.

- Guard the percentage math so an empty environment reports `[100.00%]` instead of
  `[NaN%]`.
- For the `print` action, report the assets that are eligible for garbage
  collection ("N assets (X MiB) eligible for deletion") instead of the misleading
  "0 tagged, 0 deleted" line.

Added unit tests for the progress printer covering the NaN guard, a known-total
percentage, and the print-action eligible-asset messaging.

Fixes aws#625
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.13%. Comparing base (7ff50e7) to head (840603f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1873      +/-   ##
==========================================
+ Coverage   91.10%   91.13%   +0.03%     
==========================================
  Files          80       80              
  Lines       12205    12205              
  Branches     1742     1744       +2     
==========================================
+ Hits        11119    11123       +4     
+ Misses       1050     1046       -4     
  Partials       36       36              
Flag Coverage Δ
suite.unit 91.13% <ø> (+0.03%) ⬆️

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.

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.

(garbage-collection): The 'print' action is misleading, suggesting that nothing will be deleted or tagged for deletion

2 participants