Skip to content

skip issue test_filecmp: Add tests for filecmp.cmp cache behavior and overflow clearing - #159077

Open
Bolotnyj wants to merge 3 commits into
python:mainfrom
Bolotnyj:filecmp-cache-tests
Open

Bolotnyj wants to merge 3 commits into
python:mainfrom
Bolotnyj:filecmp-cache-tests

Conversation

@Bolotnyj

@Bolotnyj Bolotnyj commented Oct 9, 2026

Copy link
Copy Markdown

Summary

This PR adds missing unit tests for the filecmp module to validate its caching behavior and automatic cache clearing mechanism.

Changes

  • Added test_cache_clear enhancements to verify successful cache hits and ensure consistent results without computing file properties repeatedly.
  • Added test_cmp_cache_clearing_on_overflow to ensure that filecmp._cache triggers a reset and effectively limits its size when capacity exceeds 100 entries.

Tested locally using coverage, ensuring full test coverage of the internal cache conditional blocks.

@python-cla-bot

python-cla-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

CLA not signed

@bedevere-app bedevere-app Bot added awaiting review tests Tests in the Lib/test dir labels Oct 9, 2026
@bedevere-app

bedevere-app Bot commented Oct 9, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@Bolotnyj Bolotnyj changed the title No issue: test_filecmp: Add tests for filecmp.cmp cache behavior and … skip issue: test_filecmp: Add tests for filecmp.cmp cache behavior and overflow clearing Oct 9, 2026
@Bolotnyj

Bolotnyj commented Oct 9, 2026

Copy link
Copy Markdown
Author

I plan to expand this work further to achieve 100% test coverage for this module. Ready for your review and workflow approval! 🙏

@BHUVANSH855 BHUVANSH855 left a comment

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.

Sign CLA to get review process started.

@Bolotnyj Bolotnyj changed the title skip issue: test_filecmp: Add tests for filecmp.cmp cache behavior and overflow clearing skip issue test_filecmp: Add tests for filecmp.cmp cache behavior and overflow clearing Oct 10, 2026
@Bolotnyj

Copy link
Copy Markdown
Author

Hello, mister @BHUVANSH855! I have already signed the CLA (confirmed on cla.python.org) and fixed the PR title format. However, the bots seem to be stuck and haven't updated the status checks yet. Could you please trigger the workflow run? Thank you!

@picnixz

picnixz commented Oct 10, 2026

Copy link
Copy Markdown
Member

Who is Stepa? this is the account that made the changes in 1dc17d0: https://github.com/stepa

@picnixz picnixz left a comment

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.

This PR is not enough. If you want to expand tests, please do it all at once rather than just a single test and please leave existing tests alone.

Comment thread Lib/test/test_filecmp.py
"Mismatched file to shallow identical file compares as equal")

def test_cache_clear(self):
first_compare = filecmp.cmp(self.name, self.name_same, shallow=False)

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.

Why changing this?

Comment thread Lib/test/test_filecmp.py


if __name__ == "__main__":
unittest.main()

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.

And this?

@bedevere-app

bedevere-app Bot commented Oct 10, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

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

Labels

awaiting changes tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants