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

Tests: all test classes should be either final or abstract #254

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Jan 11, 2024

Description

Declare all test classes as final.

Classes belonging to the test framework have been exempted at this time.

This may be revisited at a later point in time when addressing issue #25.

Note: by rights, the same should be done for most other classes, but as sniffs are often extended (even though that's not a good idea in most cases), that would be a breaking change, so will have to wait until a later point in time.

Suggested changelog entry

N/A

Classes belonging to the test _framework_ have been exempted at this time.

This may be revisited at a later point in time when addressing issue 25.
@jrfnl jrfnl added this to the 3.8.x Next milestone Jan 11, 2024
@jrfnl
Copy link
Member Author

jrfnl commented Jan 11, 2024

N.B.: PHPCS itself does not contain a sniff to enforce this, though there are various external standards which do have sniffs available. So for now, this will not be enforced via the PHPCS native ruleset and some clean up once in a while for newly introduced test classes may be needed.

@jrfnl jrfnl merged commit ed6cff1 into master Jan 11, 2024
46 checks passed
@jrfnl jrfnl deleted the feature/tests-final-or-abstract branch January 11, 2024 23:06
@fredden
Copy link
Member

fredden commented Jan 12, 2024

N.B.: PHPCS itself does not contain a sniff to enforce this, though there are various external standards which do have sniffs available. So for now, this will not be enforced via the PHPCS native ruleset and some clean up once in a while for newly introduced test classes may be needed.

Would you be open to a pull request which adds an external standard to require-dev in composer.json (and relevant changes to phpcs.xml.dist) to achieve this?

@jrfnl
Copy link
Member Author

jrfnl commented Jan 12, 2024

@fredden Not at this time, but let's discuss this further in #155.

@jrfnl
Copy link
Member Author

jrfnl commented Jan 12, 2024

@fredden Note: we could even consider - for the time being - running a few external sniffs via CI only. Adding external standards as dev dependencies will complicate things, so for now, that's not on the table.

@jrfnl jrfnl modified the milestones: 3.8.x Next, 3.9.0 Jan 19, 2024
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