Repository navigation
Conversation
…overflow clearing
|
The following commit authors need to sign the Contributor License Agreement: |
|
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 |
Bolotnyj
changed the title
|
I plan to expand this work further to achieve 100% test coverage for this module. Ready for your review and workflow approval! 🙏 |
BHUVANSH855
left a comment
BHUVANSH855
left a comment
There was a problem hiding this comment.
Sign CLA to get review process started.
Bolotnyj
changed the title
|
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! |
|
Who is Stepa? this is the account that made the changes in 1dc17d0: https://github.com/stepa |
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
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.
| "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) |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
|
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 |
|
Please sign the CLA. @picnixz I suggest we don't even review without a signed CLA, I was burned like this before where it turned out an autonomous agent who actually can't sign the CLA opened a PR... |
@StanFromIreland, same as i also proposed/suggested the workflow improvement here, I suggest we don't even open these PRs, because even if we open and review, we can't sign CLA on the behalf of the PR author. In my opinion labels will help saving reviewers time, if the labels like rather than closing the PR, we can automate our bot to draft these PRs automatically after a desired period of time only if the CLA is not signed. I also seen the older issue by Hugo, where Oleg pointed that it will be a visual clutter, but for this we can modify like the bot will add only For more discuss on here. |
|
Bhuvansh, I don't think visibility is the issue here, I find it clear already from the comment and the failing CI check. |
4 participants
Summary
This PR adds missing unit tests for the
filecmpmodule to validate its caching behavior and automatic cache clearing mechanism.Changes
test_cache_clearenhancements to verify successful cache hits and ensure consistent results without computing file properties repeatedly.test_cmp_cache_clearing_on_overflowto ensure thatfilecmp._cachetriggers 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.