Skip to content

Conversation

@fferegrino
Copy link
Contributor

This PR introduces a glob caching mechanism to improve the performance of file discovery in the CLI, particularly for large directories and repeated glob operations. It also adds comprehensive tests to ensure correct cache behavior and performance gains.

  • Added unit tests in tests/unit/test_cli_cache.py to verify correct cache key construction, separation of entries, cache clearing, and correct results for both glob and rglob operations.
  • Added end-to-end tests in tests/e2e/test_cli.py to measure and assert performance improvements from caching, including a slow test to compare cached vs. uncached runs for exclude patterns.

Checklist

  • Add test cases to all the changes you introduce
  • Update the documentation for the changes – not needed, as the change is internal and does not affect any user-facing API

@fferegrino
Copy link
Contributor Author

A similar goal but with different implementation as #294

@fferegrino
Copy link
Contributor Author

Also, the CI is failing due to Python 3.8 not being available to the "Set up Python" step.

How much of this compatibility are you planing on maintaining?

Copy link
Owner

@lyz-code lyz-code left a comment

Choose a reason for hiding this comment

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

Thanks @fferegrino for the detailed PR.

We can safely drop 3.8 as it's deprecated for a while, we will be able to drop 3.9 soon too.

Can you please tweak the CI to include this change?

@fferegrino
Copy link
Contributor Author

Can you please tweak the CI to include this change?

Sure, let me have a look.

@fferegrino
Copy link
Contributor Author

I've opened #299, hopefully this will get the tests rolling again.

@fferegrino fferegrino marked this pull request as draft September 1, 2025 20:05
It seems that the runners are slower
@fferegrino fferegrino marked this pull request as ready for review September 4, 2025 18:58
@fferegrino
Copy link
Contributor Author

Ok @lyz-code, this one seems good to be reviewed.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 17473869987

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 99.012%

Totals Coverage Status
Change from base Build 17382512253: 0.03%
Covered Lines: 501
Relevant Lines: 506

💛 - Coveralls

@fferegrino fferegrino requested a review from lyz-code September 5, 2025 19:34
@lyz-code
Copy link
Owner

lyz-code commented Sep 5, 2025

Thank you so much for contributing this feature and to clean up technical debt <3

@lyz-code lyz-code merged commit 705627e into lyz-code:main Sep 5, 2025
4 checks passed
@lyz-code
Copy link
Owner

lyz-code commented Sep 5, 2025

Available since 1.18.0

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants