Skip to content

Refactor IsCamelCapsTest to use data providers and add more tests #1147

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

Conversation

benno5020
Copy link
Contributor

Description

I've refactored the existing tests to use data providers and added more tests to document the existing behaviour. See #846

Suggested changelog entry

n/a

Related issues/external references

Fixes #846

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@benno5020 Thank you so much for this PR! Great work!

The remarks I have are only nitpicks and I'd be happy to fix these up myself when merging if you prefer (just let me know).

@benno5020
Copy link
Contributor Author

Thank you for the kind words! :)
When looking through the code again, I noticed that one test method's name and DocBlock were a little confusing so I changed it as well. See 755e941.

@jrfnl jrfnl added this to the 3.13.3 milestone Jun 26, 2025
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@benno5020 Thank you for making those updates. All good from me 👍🏻

If you don't mind, I'll reorganize the commits to be atomic again, force push and then I'll merge once the build has passed.

When looking through the code again, I noticed that one test method's name and DocBlock were a little confusing so I changed it as well. See 755e941.

Agreed, that makes the description less confusing.

This commit moves all existing test cases to data providers and adds descriptive names to better convey what they intend to test.

See PHPCSStandards#846
This commit aims to further document existing behaviour and tries to anticipate potential mistakes if the regular expressions are ever changed.

See PHPCSStandards#846
@jrfnl jrfnl force-pushed the refactor/refactor-IsCamelCapsTest-to-use-data-providers branch from 755e941 to ff8e0e0 Compare June 26, 2025 22:21
@jrfnl jrfnl merged commit 368817d into PHPCSStandards:master Jun 26, 2025
48 checks passed
@jrfnl
Copy link
Member

jrfnl commented Jun 26, 2025

@benno5020 We normally mention people by their name in the changelog. Under what name would you like me to attribute your contribution ? If you prefer not to share your name, I can use your handle instead. Just let me know your preference.

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.

Refactor IsCamelCapsTest to use dataproviders
2 participants