Testing Best Practices Skill

[nexxai]

Description update by @pushpak1300

Currently, Boost has a few different testing guidance/skills:

• pest-testing
• enforce-testing
• phpunit-guidelines

They all overlap in different ways, which makes the current testing guidance a bit hit or miss.

We've also had feedback that agents sometimes go too far with tests. They may add tests for tiny or unnecessary changes, over-mock things, test framework behavior or config that doesn't really need testing, or generally write more tests than the change actually requires.

To improve this, we're consolidating the existing guidance into a new test-best-practices skill, similar to laravel-best-practices.

The goal is to give agents better guidance on what should be tested, how much testing is appropriate, and how to write effective tests for Laravel applications without over-testing.

The skill will also be composed based on the packages installed in the project. For example, it can include specific guidance for:

• Pest
• PHPUnit
• browser testing
• TIA and other testing-related plugins

This should give us one consistent testing philosophy while still adapting the guidance to each application's actual testing stack.

I ended up running new skill on https://github.com/pinkary-project/pinkary.com and found this issue which imo are validate the issues.

Details

# Test Suite Audit

661 tests, 137 files, ~12.7k lines, reviewed against .claude/skills/testing-best-practices.
A report — nothing was changed. Per rules/review.md a duplicate shrinks at the higher layer to the
one case that proves the wiring, it is not deleted; a pattern the project repeats is a convention,
named once rather than per file.

Fix first — a real defect ships green today

#	Where	Gap
1	app/Models/User.php:97	canAccessPanel() has no test at all. Its hasVerifiedEmail() branch and second admin (mrpunyapal@gmail.com) are uncovered, while 5 Citadel HTTP files repeat the same 2-case matrix by hard-coding an email. → matrix into UserTest.php, one refused-role HTTP test per entry point.
7	livewire/questions/show.blade.php:58	{!! $question->content !!} renders ParsableContent output raw; Unit/Services/ContentTest.php feeds it zero hostile inputs. StripProviderParsable is the only guard and nothing asserts it. → add a case shaped like BioTest.php:12.
6b	Unit/Models/QuestionTest.php:47-71	root/parent/children/descendants add ->where('is_ignored', false)->where('is_reported', false) (Question.php:231-266); the test only checks instance types, so deleting both clauses stays green. → assert an ignored/reported child is excluded.

Then — same defect covered twice, shrink the higher layer

#	Where	Duplicate
2	Http/Register/CreateTest.php:200-276, :333-351	32-case + 16-case username datasets run as full HTTP requests; blocked-email repeats NotBlockedAccountTest. Split is inverted — UsernameTest.php:20-32 lumps 6 inputs under one ->throws() with no message assertion. → matrix + messages down to the rule tests, one case per rule stays up.
3	Livewire/Links/IndexTest.php:247-267, PeopleToFollowTest.php:48-60	Re-test follow / unfollow / guest-redirect that Concerns/FollowableTest.php already owns.
4	Http/About/IndexTest.php + Http/Verified/IndexTest.php	Same 4 layout facts (login button, "Your Profile", terms block, version) against one shared partial.
4	Http/Bookmarks/IndexTest.php:29-45	Builds 51 users with links and questions to re-assert the cut-off PeopleToFollowRecommendationsTest owns — likely the suite's most expensive test.
5	Http/LoginTest.php:31-42	Rate-limit test strictly contains the invalid-password test at :45-56 — 6 bcrypt hashes — and never asserts assertGuest() on the 429.

Then — suite-wide hygiene

#	Finding
8	tests/Pest.php: no Http::preventStrayRequests() (0 suite-wide — a missing fake reaches the network), no Sleep::fake(), no Exceptions::fake(); RefreshDatabase over LazilyRefreshDatabase; global Storage::fake() in beforeEach(); two dead stubs (something() at :50, toBeOne() at :37).
10	52 × assertStatus(200|302|403|404|429) across 20 files instead of assertOk()/assertForbidden()/assertNotFound()/assertTooManyRequests(). Plus Citadel/Users/IndexTest.php:24 uses assertDatabaseHas where assertModelExists() applies.
11	45 names give no result — 23 are literally 'guest'/'auth', the rest bare nouns ('relations', 'link', 'mention'). 11 files mix it() and test(); worst is Livewire/Questions/CreateTest.php (1 vs 50).
12	Factory records built in beforeEach(): Livewire/Questions/EditTest.php:12, CreateTest.php:20-21, Http/Profile/ShowTest.php:11.
14	Http/Register/CreateTest.php:302-325: AAA comments instead of blank lines, a // or specific route if expected leftover, a destination-less assertRedirect(), and expect($user)->not->toBeNull() after assertDatabaseHas proved it.

Your call — repeated conventions, not fixes

#	Pattern	Cost
6a	test('relations') in 6 model files asserting toBeInstanceOf on unconstrained relations	Tests Eloquent, not this project.
13	8 × toMatchSnapshot(), 4 of them the whole of ContentTest.php	States no contract; an XSS regression surfaces as a diff a reviewer may bless.
9	9 × expect(true)->toBeTrue() across all 7 Unit/Rules/* files	The real check is $this->fail() in the closure; reads as if nothing is asserted.

Checked and correct — no action

arch() tests (judged by the convention they protect, not behavioral value) · the 9 test('to array')
model tests (rules/isolation.md endorses asserting exact toArray() keys; each uses toHaveCount()) ·
BCRYPT_ROUNDS=4 in phpunit.xml · bio XSS escaping at Unit/Services/BioTest.php:12.

Note

Once merged we need to delete this skill

[jackwh]

I think it should be Http::preventStrayRequests() (which prevents them), rather than Http::preventingStrayRequests() (which returns a bool) 👀 FYI!