Skip to content

Commit fbdc50d

Browse files
committed
Clarify Testing Convention and Coverage Guidance
Signed-off-by: Pushpak Chhajed <pushpak1300@gmail.com>
1 parent 9fa9ff5 commit fbdc50d

4 files changed

Lines changed: 54 additions & 6 deletions

File tree

.ai/laravel/skill/testing-best-practices/SKILL.blade.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919

2020
Read nearby tests before you choose syntax and organization.
2121

22-
Follow established conventions when they preserve the behavior required by this skill.
22+
A pattern that the project repeats is a convention, and a convention of the project outranks each rule in this skill. Follow it, and write the new test in the same shape.
23+
24+
The rules in this skill govern the test that you write now. A test that exists and follows a convention is not a defect of that test. Do not delete it, and do not rewrite it. Tell the user what the convention costs, if it costs something, and let the user decide.
2325

2426
Use the project convention for each item that follows:
2527

@@ -39,8 +41,8 @@
3941
- Test observable behavior and application contracts. A test must pass after an implementation change if the behavior stays the same.
4042
- Cover every changed decision and each applicable high-value failure mode. A decision is a branch, a validation, a calculation, or an authorization.
4143
- Exercise declarations through behavior instead of repeating their text.
42-
- Leave framework behavior to the framework tests.
43-
- Keep each test only when it can detect a distinct defect.
44+
- Leave framework behavior to the framework tests. A test of what this project configures is not a test of the framework. A relation that adds a constraint, a cast, a scope, and a rule of the validation each belong to this project.
45+
- Keep each test that can detect a distinct defect. When two tests find the same defect, trim the test at the higher layer to one case, and report the duplicate to the user. Do not delete a test.
4446
- Write a feature test first. Write a unit test only for logic that does not use the framework.
4547
@if($pest && $assist->hasPackage('pestphp/pest-plugin-browser'))
4648
- Write a browser test only for behavior in JavaScript that a feature test cannot reach. Put a browser test in `tests/Browser`, and call `assertNoJavaScriptErrors()` in it.
@@ -49,6 +51,9 @@
4951
@else
5052
- Write a feature test for each behavior that a request can reach. A test in a real browser needs {{ $pest ? '`pestphp/pest-plugin-browser`' : '`laravel/dusk`' }} and a browser download, and this project installs neither of them. Tell the user about the package only if the user asks for a test in a real browser.
5153
@endif
54+
@if($pest)
55+
- Judge an architecture test by the convention that it protects, and not by the rules above. An `arch()` test states a rule for a complete directory, such as the parent of each model, the classes that may use an enum, or the methods that each factory declares. It is a declaration check by design, and it fails when one new file breaks the convention.
56+
@endif
5257
- Use the test tools that the project installs. Add a new test dependency, plugin, or browser only after the user asks for it.
5358

5459
## How to Apply

.ai/laravel/skill/testing-best-practices/rules/endpoint-tests.blade.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,9 @@
8484
@endif
8585

8686
Send an input value that is not valid through the application, and assert the error. Do not assert that an array of rules contains a string, because that assertion tests the declaration and not the behavior. Use such an assertion only for a rule that no request can reach, and write the reason in the test.
87+
88+
### Which Layer Owns Which Case
89+
90+
The test of a rule class owns the matrix of the values that pass and that fail. The test of the endpoint owns the proof that the endpoint applies the rule, and that the user gets the message.
91+
92+
Move the matrix to the test of the rule class when both tests hold it, and keep one case in the test of the endpoint. Never remove the last case, because the test of the rule class passes while the request forgets the rule. The same split applies to a policy, to a scope, and to any other class that a request calls.

.ai/laravel/skill/testing-best-practices/rules/review.blade.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,17 @@
66

77
Check every item in this file. A test that passes can have no value. For each test, state the defect that the test finds.
88

9+
A review reports. Report each item that you find, and do not delete a test and do not rewrite a test without approval of the user. An item that the project repeats throughout the suite is a convention, and the report names the pattern once instead of each file that follows it.
10+
911
## The Value of the Test
1012

13+
@if($pest)
14+
Apply this section to a test of behavior. An architecture test states a convention for a directory, and the items that follow do not apply to it.
15+
16+
@endif
1117
- [ ] Each test covers observable behavior or an application contract, and passes after a change to the implementation that keeps the behavior.
12-
- [ ] Each tested declaration is exercised through behavior, and no test asserts the behavior of the framework.
13-
- [ ] Each test detects a distinct defect that no other test covers.
18+
- [ ] Each tested declaration is exercised through behavior, and no test asserts the behavior of the framework. A test of what this project configures, such as a relation with a constraint, a cast, or a scope, belongs to this project.
19+
- [ ] Each test detects a distinct defect that no other test covers. A duplicate shrinks at the higher layer to the one case that proves the wiring.
1420
- [ ] Every changed decision and each applicable high-value failure mode has coverage.
1521

1622
## The Names and the Structure
@@ -28,7 +34,7 @@
2834
- [ ] The HTTP tests cover the authentication, the authorization, the role, the scope, and the validation, if the case applies.
2935
- [ ] A request for a record of a different tenant gets a status code that does not confirm that the record exists.
3036
- [ ] The complete matrix of the permissions is in the tests of the policy, and not in the tests of the controller.
31-
- [ ] Each rule of the validation has one test, and the test asserts the message that the user gets.
37+
- [ ] Each rule of the validation has one test, and the test asserts the message that the user gets. A duplicate of a matrix that a unit test owns shrinks to one case, and it is not deleted.
3238
- [ ] The output of a user and each dynamic part of a query have a security test.
3339

3440
## The Data and the Determinism

tests/Feature/Install/TestingSkillsTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,37 @@ public function render(string $path): string
100100
'phpunit with the pest plugin, which needs pest' => [false, ['pestphp/pest-plugin-browser'], 'laravel/dusk'],
101101
]);
102102

103+
it('puts a convention of the project above its own rules, and never deletes a test without approval', function (bool $pest): void {
104+
expect(renderTestingSkill($pest))
105+
->toContain('a convention of the project outranks each rule in this skill')
106+
->toContain('Do not delete it, and do not rewrite it.')
107+
->toContain('do not delete a test and do not rewrite a test without approval of the user');
108+
})->with([
109+
'pest' => true,
110+
'phpunit' => false,
111+
]);
112+
113+
it('keeps one case at the endpoint when a unit test owns the matrix', function (bool $pest): void {
114+
expect(renderTestingSkill($pest))
115+
->toContain('### Which Layer Owns Which Case')
116+
->toContain('Never remove the last case')
117+
->toContain('shrinks to one case, and it is not deleted')
118+
->toContain('trim the test at the higher layer to one case')
119+
->toContain('A test of what this project configures is not a test of the framework.');
120+
})->with([
121+
'pest' => true,
122+
'phpunit' => false,
123+
]);
124+
125+
it('exempts an architecture test from the value rules, which only Pest can write', function (): void {
126+
expect(renderTestingSkill(pest: true))
127+
->toContain('Judge an architecture test by the convention that it protects')
128+
->toContain('the items that follow do not apply to it');
129+
130+
expect(renderTestingSkill(pest: false))
131+
->not->toContain('architecture test');
132+
});
133+
103134
it('teaches a Pest 5 command only to a project that installs Pest 5', function (): void {
104135
expect(renderTestingSkill(pest: true, version: '5.0.0'))
105136
->toContain('pest --parallel --tia')

0 commit comments

Comments
 (0)