Skip to content

Avoid scanning the filesystem to filter files per check#1262

Open
Wigny wants to merge 2 commits into
rrrene:masterfrom
Wigny:fix/avoid-filesystem-scan-in-check-runner
Open

Avoid scanning the filesystem to filter files per check#1262
Wigny wants to merge 2 commits into
rrrene:masterfrom
Wigny:fix/avoid-filesystem-scan-in-check-runner

Conversation

@Wigny
Copy link
Copy Markdown
Contributor

@Wigny Wigny commented Feb 13, 2026

We started noticing credo became much slow latelly when trying to execute checks that set files.included glob patterns like the Credo.Check.Refactor.PassAsyncInTestCases.

This slowdown seems to be caused, according to my test, by Credo.Sources.find_in_dir/3, which uses Path.wildcard to list matching file paths from the filesystem, which might be running a recursive scan of the entire project tree.

This PR changes the code to filter the already known file paths in memory using Credo.Sources.filename_matches? instead of listing them from the filesystem.

Tested it in our project, and the slowdown was reduced from several minutes to seconds.

@rrrene
Copy link
Copy Markdown
Owner

rrrene commented Feb 14, 2026

First, thx for the PR!

I don't have access to my dev machine at the moment, but I am reasonably sure that this change will cause problems when piping files via STDIN (via --read-from-stdin) or using the watcher (via --watch).

I will have to verify if these scenarios are sufficiently covered by tests right now (and improve the test suite if not) 👍

@Wigny Wigny force-pushed the fix/avoid-filesystem-scan-in-check-runner branch from de901fe to 326a676 Compare March 3, 2026 19:55
@Wigny
Copy link
Copy Markdown
Contributor Author

Wigny commented Mar 3, 2026

Hey @rrrene, I've pushed a commit that adds some tests that I hope would help to catch problems if the behaviour of 'read from stdin' or 'watcher' changed. Let me know if that is useful or if I'm still missing something on those tests.

@Wigny Wigny force-pushed the fix/avoid-filesystem-scan-in-check-runner branch from 326a676 to 45cc6c4 Compare March 3, 2026 20:08
@Wigny
Copy link
Copy Markdown
Contributor Author

Wigny commented Apr 16, 2026

Hey @rrrene, let me know if you want me to convert this into an issue and close this PR, or if you would like me to rebase this with master and resolve the conflicts.

@rrrene
Copy link
Copy Markdown
Owner

rrrene commented Apr 16, 2026

@Wigny sorry for the slow progress on this, I am just really concerned about regressions.

We should probably think about re-architecting this functionality, since you correctly raised the fact that for single-run invocations of mix credo (without reading from STDIN or a file watcher), this could be a lot simpler and faster.

Let's look up where this function is called from and if we could refactor it to receive an exec struct (so can have a bit context).

@Wigny
Copy link
Copy Markdown
Contributor Author

Wigny commented Apr 16, 2026

Hey @rrrene, no problem!

I am just really concerned about regressions.

That makes a lot of sense and is completely reasonable.

if we could refactor it to receive an exec struct

I could try making this refactor, but I wanted to let you know that this MR was more of an attempt to solve the problem we faced, hoping you can tell if it was going in the right direction or not, since I'm also not 100% sure that those changes will not introduce any undesired behaviour.

Therefore, let me know if you want me to pursue this refactor, or if you would like to address it.

Again, no need to rush on this, and thanks for maintaining this awesome lib!

@Wigny Wigny force-pushed the fix/avoid-filesystem-scan-in-check-runner branch from 45cc6c4 to 75c0789 Compare April 27, 2026 14:37
Checks with custom :files patterns previously triggered a recursive scan of
the entire project tree on every check run.

The filtering is now done in-memory against already-loaded source files.
This also makes the STDIN special case unnecessary.
@Wigny Wigny force-pushed the fix/avoid-filesystem-scan-in-check-runner branch from 75c0789 to 3241379 Compare April 27, 2026 17:37
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.

2 participants