Common::getSniffCode(): add tests, more defensive coding and minor simplification #524
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
7 tasks
fredden approved these changes Jun 7, 2024
jrfnl force-pushed the feature/common-add-tests-and-simplify branch from 86982b0 to 659becb Compare 
| Rebased without changes. |
Add initial set of tests for the `Common::getSniffCode()` method. Related to 146 Related to [review comment in PR 446](#446 (comment)).
Previously, if an empty string was passed, the `Common::getSniffCode()` method would return `..`, which is just confusing (and incorrect). This commit changes the behaviour to throw an `InvalidArgumentException` instead. Includes making a potentially superfluous function call to the method conditionally (as it could hit the new exception). Includes test.
Previously, if an invalid (incomplete) class name was passed, the `Common::getSniffCode()` method would return a garbled name, like `.Qualified.C`, which is just confusing. This commit changes the behaviour to throw an `InvalidArgumentException` instead. Includes test.
Previously, if an invalid class name was passed, which didn't end on `Sniff` or `UnitTest`, the `Common::getSniffCode()` method would return a garbled name, like `Fully.Qualified.C`, which is just confusing. This commit changes the behaviour to throw an `InvalidArgumentException` instead. Includes test.
* Remove the use of `array_pop()` in favour of directly referencing the required "parts" by their index in the array. * Remove the unused `$sniffDir` variable. * Remove the unnecessary `$code` variable. Related to [review comment in PR 446](#446 (comment)).
jrfnl force-pushed the feature/common-add-tests-and-simplify branch from 659becb to 7449b29 Compare This was referenced Oct 3, 2024
| Forwarded to #675 |
| @williamdes I'd need to see the code of the |
Sure, thank you for responding so quickly. |
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Description
Common::getSniffCode(): add tests
Add initial set of tests for the
Common::getSniffCode()method.Related to #146
Related to review comment in PR 446.
Common::getSniffCode(): throw exception on invalid input [1]
Previously, if an empty string was passed, the
Common::getSniffCode()method would return.., which is just confusing.This commit changes the behaviour to throw an
InvalidArgumentExceptioninstead.Includes making a potentially superfluous function call to the method conditionally (as it could hit the new exception).
Includes test.
Common::getSniffCode(): throw exception on invalid input [2a]
Previously, if an invalid (incomplete) class name was passed, the
Common::getSniffCode()method would return a garbled name, like.Qualified.C, which is just confusing.This commit changes the behaviour to throw an
InvalidArgumentExceptioninstead.Includes test.
Common::getSniffCode(): throw exception on invalid input [2b]
Previously, if an invalid class name was passed, which didn't end on
SnifforUnitTest, theCommon::getSniffCode()method would return a garbled name, likeFully.Qualified.C, which is just confusing.This commit changes the behaviour to throw an
InvalidArgumentExceptioninstead.Includes test.
Common::getSniffCode(): minor simplification
array_pop()in favour of directly referencing the required "parts" by their index in the array.$sniffDirvariable.$codevariable.Related to review comment in PR 446.
Suggested changelog entry
The
Common::getSniffCode()method will now throw anInvalidArgumentExceptionexception if an invalid$sniffClassis passed.