Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruleset::registerSniffs(): add tests #756

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 7, 2024

Description

Suggested changelog entry

N/A

Related issues/external references

This PR is part of a series of PRs expanding the tests for the Ruleset class.

Copy link
Member

@fredden fredden left a comment

Choose a reason for hiding this comment

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

This looks like a good addition to the test suite. Is it intentional that this code block isn't being tested?

$slashPos = strrpos(substr($file, 0, $sniffPos), DIRECTORY_SEPARATOR);
if ($slashPos === false) {
continue;
}

I understand that the 'if verbose echo' code at the end of the loop isn't feasibly testable right now and there are plans to make it so later on.

@jrfnl
Copy link
Member Author

jrfnl commented Dec 9, 2024

@fredden Thanks for reviewing!

This looks like a good addition to the test suite. Is it intentional that this code block isn't being tested?

Kind of. My fantasy ran out and I couldn't come up with a (realistic) test case which managed to hit that line (I did try, but if I remember correctly, anything I tried which should hit the line, was already filtered out of the $files list before it reached the registerSniffs() method). It may actually be redundant code, but I didn't think removing it now was prudent. Let's give it a little more time, who knows, someone else may be able to come up with a valid test case for it ;-)

@jrfnl jrfnl added this to the 3.11.x milestone Dec 11, 2024
@jrfnl jrfnl merged commit d7ecf7e into master Dec 12, 2024
57 checks passed
@jrfnl jrfnl deleted the feature/ruleset-add-tests-registersniffs branch December 12, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants