Skip to content

Running tmt clean will now show disk space freed#4801

Open
AthreyVinay wants to merge 3 commits intomainfrom
avinay-clean-command-disk-space
Open

Running tmt clean will now show disk space freed#4801
AthreyVinay wants to merge 3 commits intomainfrom
avinay-clean-command-disk-space

Conversation

@AthreyVinay
Copy link
Copy Markdown
Member

@AthreyVinay AthreyVinay commented Apr 15, 2026

resolves: #4593

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

@AthreyVinay AthreyVinay requested a review from lbrabec as a code owner April 15, 2026 13:12
@AthreyVinay AthreyVinay added the command | clean tmt clean command label Apr 15, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements disk space reporting for the clean command by adding utilities to calculate and format file sizes. The cleaning logic for workdirs and testcloud images now logs the amount of space freed, supported by new test assertions. Feedback recommends using lstat to prevent symlink traversal during size calculation and employing floating-point arithmetic in format_size to ensure accurate unit conversion.

Comment thread tmt/base/core.py Outdated
Comment thread tmt/utils/__init__.py Outdated
Comment thread tmt/utils/__init__.py Outdated
from tmt.hardware import Size


def format_size(size: int) -> str:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use tmt.hardware.UNITS instead: tmt.hardware.UNITS(f'{size} bytes').to_compact()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done: c4b334a

Comment thread tmt/base/core.py Outdated
return successful

def _clean_workdir(self, path: Path) -> bool:
def _clean_workdir(self, path: Path, size: int) -> bool:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Computing the size outside of _clean_workdir and just let it print out is very weird. We can change the return value of _clean_workdir, to e.g. a tuple of (success, size). _clean_workdir would collect the size, and just tell its caller how much data it did/would remove.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Absolutely - that was my intuition too. The only reason was I was trying to avoid returning a tuple - and I thought dataclass would be an overkill. Will revert to returning a tuple instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done: c4b334a

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

Labels

command | clean tmt clean command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tmt clean --dry could show total disk space that would be freed

2 participants