Upgrade test tooling to PHPUnit 12 + add Infection/PHPStan + regression tests#1469
Upgrade test tooling to PHPUnit 12 + add Infection/PHPStan + regression tests#1469Refaltor77 wants to merge 2 commits into
Conversation
- Migrate phpunit.xml from PHPUnit 9/10 schema to PHPUnit 12:
* Drop deprecated attributes (cacheResultFile, forceCoversAnnotation,
beStrictAboutCoversAnnotation, beStrictAboutTodoAnnotatedTests,
verbose)
* Replace <coverage><include> with <source><include>
* Use cacheDirectory for unified PHPUnit cache
- Add dev dependencies for stricter CI:
* phpstan/phpstan ^2.1 (level max baseline)
* infection/infection ^0.32 (mutation testing, configured via
infection.json5, scoped filter for fast runs)
- Gitignore .papline/ (local workflow log directory)
Required to run the new InviteTest / MemberTest suites under PHPUnit 12
and to back the suite with static analysis + mutation coverage.
Locks in the behaviour of three already-merged upstream bug fixes (8258c2f, 565e53e, 35f15b6) against future regression. tests/Parts/Channel/InviteTest.php — 17 tests covering: - inviter-is-bot allows access without manage_guild / view_audit_log - non-inviter without perms is still rejected - manage_guild OR view_audit_log each individually allow access - strict comparison edge cases (leading zeros, trailing space, zero-width space) - 1 MiB id comparison stability - 100 concurrent invocations respect per-call permission state tests/Parts/User/MemberTest.php — 6 tests covering: - Null-guild path produces a rejected Promise (regression test) - Missing kick_members permission produces NoPermissionsException - Empty-string and Unicode reasons do not bypass null-guild handling - 100 concurrent kick() calls all reject (never resolve with an exception value, which was the old bug's symptom) - Explicit guard that kick() must not resolve with a Throwable value under any code path Infection MSI on Invite.php/Channel.php scope: 93% (41/43 mutations killed, 100% mutation code coverage). Infection MSI on Member.php scope: 100% (7/7 mutations killed).
alexandre433
left a comment
alexandre433
left a comment
There was a problem hiding this comment.
It just looks like AI slop; without a care in the current project's structure, psr-12 usage, and adding tools just willy nilly without really checking what we really are using already.
| * tests can inject stubbed Channel and User parts without needing the full | ||
| * factory / repository chain. | ||
| */ | ||
| class InviteTestStub extends Invite |
There was a problem hiding this comment.
These "stubs" should not exist as a class inside the same file that the test is in.
Either create a "fake" class with new <class>() or add these classes to separate files; If you choose to create new files, I'd suggest adding into a folder called "Stubs".
| * that is the inviter is allowed through even when it lacks manage_guild | ||
| * and view_audit_log. | ||
| */ | ||
| public function testGetTargetUsersAllowsBotWhenBotIsInviter(): void |
There was a problem hiding this comment.
Why the comment on this test and not the others?
There was a problem hiding this comment.
Big advice here; from what I see in the current main code, the tests are not properly formed, if you want your tests to properly work, make the base work global, as in ready to get picked up by other classes, but that doesn't overwrite what already exists.
| * Test stub that lets us instantiate Member without running the full Part | ||
| * constructor (which requires a fully-wired Discord instance). | ||
| */ | ||
| class MemberTestStub extends Member |
There was a problem hiding this comment.
This makes even less sense to me because the test suite does have support for creating a mock instance of the Discord client, so it's already possible to do (and other tests do this currently)
There was a problem hiding this comment.
Same as before with the testing structure, stubs and comments.
| /.phpunit* | ||
| /coverage | ||
| /.tools | ||
| .papline/ |
There was a problem hiding this comment.
Follow other files'/folders' structure /.papline
| "carthage-software/mago": "1.1.0" | ||
| "carthage-software/mago": "1.1.0", | ||
| "phpstan/phpstan": "^2.1", | ||
| "infection/infection": "^0.32.6" |
There was a problem hiding this comment.
Remove unnecessary packages;
phpstan is already imported via laravel/pint which imports larastan/larastan which imports phpstan.
|
Converting to draft. |
3 participants
Summary
Follow-up to #1468 (closed) — that PR mixed bug fixes (now all merged upstream via
8258c2ff,565e53ea,35f15b65) with tooling/tests. Per the review comment asking to split bug fixes and test enhancements into separate PRs, this PR contains only the test-tooling upgrade and the unit tests that lock in those three fixes.Two commits, each reviewable independently:
1.
chore(test): upgrade phpunit.xml to PHPUnit 12 + add phpstan/infectionphpunit.xmlfrom the PHPUnit 9/10 schema to PHPUnit 12 (drop deprecated attributes, switch<coverage><include>to<source><include>, unifiedcacheDirectory).phpstan/phpstan ^2.1andinfection/infection ^0.32as dev dependencies + a minimalinfection.json5config scoped for fast runs..papline/(local workflow log directory).2.
test: add InviteTest / MemberTest regression + adversarial suitestests/Parts/Channel/InviteTest.php— 17 tests covering theisInviterprecedence regression (bot-is-inviter bypass, non-inviter rejection,manage_guild/view_audit_logexemptions) plus adversarial cases (leading zeros, trailing space, zero-width space, 1 MiB ids, 100 concurrent invocations).tests/Parts/User/MemberTest.php— 6 tests coveringMember::kick()null-guild path (regression: must return a rejected Promise, not resolve with a Throwable value), missing-permission path, empty/Unicode reasons, 100 concurrent calls.Test plan
./vendor/bin/phpunit— new tests pass against currentmaster./vendor/bin/pint --test— pass on modified files./vendor/bin/mago lint— zero regression vs baseline./vendor/bin/phpstan --level=max— zero regression on touched files./vendor/bin/infection— MSI 93% onInvite.php/Channel.phpscope, MSI 100% onMember.phpscope (threshold 85%)Note
No source files under
src/are modified in this PR. All three underlying fixes these tests cover are already present onmaster.