NEW Use symfony/finder to perform file lookups in checks #10
Conversation
robbieaverill
force-pushed
the
pulls/1.0/filesystem
branch
from
e6f23b4
to
0291ddb
Compare
src/Check/CodeOrSrcFolderCheck.php
Outdated
| ->in($this->getSuite()->getModuleRoot()) | ||
| ->name('/code|src$/'); | ||
|
|
||
| $this->setSuccessful(count($files) === 1); |
There was a problem hiding this comment.
If you have both code and src this will fail. It used to pass. Is that intentional?
There was a problem hiding this comment.
It was intentional because I figured you wouldn't have either, but I suppose that PSR-4 could be pointing at src while devs could have something else in code for any reason, and that's valid. I'll update this and the test
| $files = $this->getFinder() | ||
| ->files() | ||
| ->in($this->getSuite()->getModuleRoot()) | ||
| ->name('/contributing(?:\.md|\.txt)?$/i'); |
There was a problem hiding this comment.
We only have a smoke test that doesn't test the different paths of this. Perhaps a unit test to check the regex reliability would be good?
There was a problem hiding this comment.
Updated, see AbstractFileCheckTest
src/Check/DocumentationCheck.php
Outdated
| ->in($this->getSuite()->getModuleRoot()) | ||
| ->name('docs'); | ||
|
|
||
| $this->setSuccessful(count($files) > 0); |
There was a problem hiding this comment.
Should we allow "doc" as equally valid?
| break; | ||
| } | ||
| } | ||
| $files = $this->getFinder() |
There was a problem hiding this comment.
We only have a smoke test that doesn't test the different paths of this. Perhaps a unit test to check the regex reliability would be good? I see you've done this for 'license'.
There was a problem hiding this comment.
Yeah that's right - I only wrote the one test case because the logic in the readme and contributing check is the same thing as this one, so I've marked ReadmeCheckTest::testRun as covering the other methods too. If you think its worthwhile I could duplicate the test for those cases as well?
There was a problem hiding this comment.
Updated, moved to AbstractFileCheckTest with individual cases for each file check's regex
This implements the symfony/finder component to replace manual filesystem checks, and uses regex patterns to reduce some duplication.
Resolves #2