-
-
Notifications
You must be signed in to change notification settings - Fork 83
Adopt Boost testing skill #452
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| --- | ||
| name: testing-best-practices | ||
| description: "Laravel test design and review. Use when selecting coverage, naming or structuring tests, choosing assertions or test data, isolating dependencies, testing HTTP or security boundaries, improving suite performance, or reviewing test value. Use framework guidance or search-docs for Pest and PHPUnit syntax." | ||
| license: MIT | ||
| metadata: | ||
| author: laravel | ||
| --- | ||
|
|
||
| # Testing Best Practices | ||
|
|
||
| This skill provides rules for designing Laravel tests. Each rule file explains what to do and why. Use `search-docs` for Laravel and Pest API syntax. | ||
| This project uses Pest. Follow the corresponding guidance in each rule. | ||
|
|
||
| ## Consistency First | ||
|
|
||
| Read nearby tests before you choose syntax and organization. | ||
|
|
||
| A pattern repeated throughout the project is a convention, and project conventions take precedence over this skill. Follow them and give new tests the same structure. | ||
|
|
||
| These rules govern the tests you write now. An existing test that follows a project convention is not defective merely because it conflicts with this skill. Do not delete or rewrite it. If the convention has drawbacks, explain them and let the user decide. | ||
|
|
||
| Use the project convention for each item that follows: | ||
|
|
||
| - the use of `it()` or `test()` | ||
| - the construction of a factory | ||
| - the setup of the authentication | ||
| - the layout of the files | ||
|
|
||
| ## What to Test | ||
|
|
||
| Read this section before you write a test. | ||
|
|
||
| - Test observable behavior and application contracts. A test must pass after an implementation change if the behavior stays the same. | ||
| - Cover every changed decision and each applicable high-value failure mode. A decision is a branch, a validation, a calculation, or an authorization. | ||
| - Exercise declarations through behavior instead of repeating their text. | ||
| - Leave framework behavior to framework tests. Testing project configuration is not testing the framework. A constrained relationship, cast, scope, or validation rule belongs to this project. | ||
| - Keep every test that can detect a distinct defect. When two tests detect the same defect, trim the higher-layer test to one case and report the duplication. Do not delete an existing test. | ||
| - Write a feature test first. Write a unit test only for logic that does not use the framework. | ||
| - Write a feature test for every behavior reachable through a request. Real-browser tests require `pestphp/pest-plugin-browser` and a browser download, neither of which this project installs. Mention the package only if the user asks for a real-browser test. | ||
| - Judge an architecture test by the convention it protects, not by the rules above. An `arch()` test declares a rule for an entire directory, such as the parent class of every model, the classes that may use an enum, or the methods every factory declares. It intentionally checks declarations and fails when a new file breaks the convention. | ||
| - Use the test tools that the project installs. Add a new test dependency, plugin, or browser only after the user asks for it. | ||
|
|
||
| ## How to Apply | ||
|
|
||
| 1. Read the code under test. Read the tests in the same directory. Identify every decision in the code. | ||
| 2. Select every applicable branch in the rule index. Read every selected rule file. | ||
| 3. Report each defect in the code before you write a test. Examples are a method with no body, a policy that no action calls, and a write action with no validation. Test the actual behavior. Report the defect to the user. | ||
| 4. Write the tests. Run the smallest set of tests that covers the change. The tests must pass. | ||
| 5. Check every applicable item in `rules/review.md` and every selected rule file. Resolve every mismatch before completion. | ||
|
|
||
| ## Rule Index | ||
|
|
||
| Most changes need more than one rule file. | ||
|
|
||
| | Subject | Rule file | | ||
| | --- | --- | | ||
| | A feature of the test framework that can already do the work | [`rules/finding-features.md`](rules/finding-features.md) | | ||
| | The layout of the files, the names of the tests, and the groups | [`rules/naming.md`](rules/naming.md) | | ||
| | Arrange-act-assert, and the correct assertion for each subject | [`rules/assertions.md`](rules/assertions.md) | | ||
| | The coverage of an endpoint, the authentication, the authorization, the isolation of a tenant, the validation, and the tests in a browser | [`rules/endpoint-tests.md`](rules/endpoint-tests.md) | | ||
| | The factories, the owner of the test data, and the repeated input values | [`rules/test-data.md`](rules/test-data.md) | | ||
| | The fakes, the mocks, the outbound HTTP, the time, the randomness, and the database | [`rules/isolation.md`](rules/isolation.md) | | ||
| | The escaping, the injection, the access across tenants, and the checks of privilege | [`rules/security.md`](rules/security.md) | | ||
| | The settings of the environment and of the CI for a slow suite | [`rules/performance.md`](rules/performance.md) | | ||
| | The review of a test or of a suite | [`rules/review.md`](rules/review.md) | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # Assertions | ||
|
|
||
| ## Arrange, Act, Assert | ||
|
|
||
| Write each test in three parts: setup, one action, and assertions. Put one blank line between them so readers can identify each part without comments. | ||
|
|
||
| Keep each test self-contained. Do not use values created by another test. | ||
|
|
||
| ## How to Find the Correct Assertion | ||
|
|
||
| First identify the subject of the check, then find an assertion designed for it. A subject-specific assertion identifies the incorrect value when the test fails. | ||
|
|
||
| 1. Search Laravel's assertions for framework subjects such as responses, the database, sessions, models, queues, events, mail, and notifications. | ||
| 2. Fetch `https://pestphp.com/docs/expectations.md` for the expectations of Pest for a plain value, a type, a format, or a shape. | ||
| 3. Build the check by hand only if no assertion exists for the subject. | ||
| 4. Confirm the name in the documentation before you use it. Do not write an assertion that you did not confirm. | ||
|
|
||
| Use the assertion in this table for each subject. | ||
|
|
||
| | Subject | Assertion to use | | ||
| | --- | --- | | ||
| | A return value, the state of an object, or a transformation of a value | an `expect()` chain | | ||
| | An HTTP status, JSON, a session, or Inertia | a Laravel response assertion | | ||
| | The state in the database | a Laravel database assertion | | ||
| | The existence of a model | `assertModelExists($model)` rather than `assertDatabaseHas('users', ['id' => $user->id])` | | ||
|
|
||
| Use a PHPUnit assertion only if no Pest expectation and no Laravel assertion exists for the subject. | ||
|
|
||
| Assert each fact once. Do not assert a 200 status before `assertSee`, because `assertSee` already shows that the page rendered. | ||
|
|
||
| ## The Assertion with a Name for a Response | ||
|
|
||
| Use a named response assertion, such as `assertNotFound()`, rather than `assertStatus(404)`. A failure then identifies the broken contract. Laravel provides named assertions for commonly tested status codes. | ||
|
|
||
| Keep one `expect()` chain on one subject. Start a new chain when the subject changes, or when the chain is difficult to read. | ||
|
|
||
| ## Assert a Known Value | ||
|
|
||
| Write the expected value in the test, or calculate the expected value by a different method. Do not calculate the expected value with the logic of the implementation, because the test then passes when that logic is wrong. | ||
|
|
||
| ```php | ||
| // The test calculates the value with the logic of the implementation. | ||
| $expected = now()->subHours(24)->floorSeconds(30)->toJson(); | ||
| expect($from)->toBe($expected); | ||
|
|
||
| // The test sets a fixed input and asserts a known value. | ||
| travelTo('2025-01-01 00:00:00'); | ||
| expect($from)->toBe('2024-12-31T00:00:00.000000Z'); | ||
| ``` | ||
|
|
||
| ## Assert the Complete Result | ||
|
|
||
| A status code is not the complete result of a write operation. Assert each of the following if the operation changes it: | ||
|
|
||
| - the response or the return value | ||
| - the state in the database | ||
| - the jobs and the events that the operation dispatches | ||
| - the notifications and the mail that the operation sends | ||
|
|
||
| On the failure path, assert that the operation makes none of these changes. A test that asserts only `assertOk()` passes even when the application saves no record. | ||
48 changes: 48 additions & 0 deletions
48
.agents/skills/testing-best-practices/rules/endpoint-tests.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Endpoint Tests | ||
|
|
||
| ## How to Write the Test | ||
|
|
||
| Fetch `https://laravel.com/framework/docs/http-tests` for the request helpers, the authentication helpers, and the response assertions. Confirm the name before you use it, and do not guess an assertion. | ||
|
|
||
| Choose an assertion based on the subject of the check: the status, a header, a redirect, the JSON body, the session, a validation error, or the view. Laravel provides a named assertion for each subject that identifies the incorrect value. | ||
|
|
||
| ## The Coverage of an Endpoint | ||
|
|
||
| Write a test for each applicable case: | ||
|
|
||
| - The request has missing or invalid authentication. | ||
| - The request comes from a different tenant, team, or organization. | ||
| - The user has an insufficient role or permission. | ||
| - The request does not satisfy a route or scope constraint. | ||
| - The request fails the validation. | ||
| - The request is valid. Assert both the response and the persisted state. | ||
|
|
||
| Assert the application's actual behavior rather than a generic status code. An API returns `401` for a missing or invalid token, while a browser endpoint redirects to the sign-in route. | ||
|
|
||
| ## The Isolation of a Tenant | ||
|
|
||
| Assert the status code returned for a cross-tenant request. Use `404` rather than `403` when one tenant must not learn that another tenant's record exists, because `403` confirms its existence. | ||
|
|
||
| ## Test Authorization at the Policy Level | ||
|
|
||
| An HTTP test shows that the endpoint performs authorization. It cannot identify which mechanism refused the request because middleware, a policy, and a call to `abort()` can all return `403`. | ||
|
|
||
| - Assert the complete matrix of the permissions against the policy or the gate. A failure then names the rule that is not correct. | ||
| - Write one HTTP test for one refused role, which shows that the endpoint calls the authorization. | ||
| - Use the helper of the project that asserts the ability and the arguments of the gate, if such a helper exists. | ||
|
|
||
| ## The Validation | ||
|
|
||
| - Write one test for each validation rule when each failure represents a separate contract. | ||
| - Write one test with an empty payload to assert several required fields together. | ||
| - Give the status code in the name of a test for an API. | ||
| - Assert the text of the message that the user gets. A message that is present but wrong is a defect. | ||
| - Use a dataset for input values that need the same setup and the same assertions. | ||
|
|
||
| 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. | ||
|
|
||
| ### Which Layer Owns Which Case | ||
|
|
||
| The rule-class test owns the matrix of values that pass and fail. The endpoint test proves that the endpoint applies the rule and that the user receives the message. | ||
|
|
||
| When both tests contain the matrix, move it to the rule-class test and retain one case in the endpoint test. Never remove the last case, because the rule-class test still passes if the request omits the rule. The same division applies to policies, scopes, and other classes called by a request. |
36 changes: 36 additions & 0 deletions
36
.agents/skills/testing-best-practices/rules/finding-features.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # How to Find Test Framework Features | ||
|
|
||
| Pest adds features faster than this skill can list them. Find an existing feature before implementing the behavior by hand. | ||
|
|
||
| - Give `search-docs` the capability you need rather than the name of a function you remember. It returns features available in the installed version. | ||
| - Fetch `https://pestphp.com/llms.txt` for the complete feature list and additions in each release. | ||
| - If a search returns no results, tell the user that the installed version does not provide the feature. Do not write an API that you have not confirmed. | ||
|
|
||
| Search for a feature in this table before you write the code by hand. | ||
|
|
||
| | Work that you need | Term to search for | | ||
| | --- | --- | | ||
| | Run one test with many input values | datasets, bound datasets | | ||
| | Assert over many values or over a collection | higher-order expectations | | ||
| | Remove the same setup from each test in a file | hooks, higher-order tests | | ||
| | Apply a convention to the complete codebase | architecture testing | | ||
| | Measure if the suite finds a defect | mutation testing | | ||
| | Find code with no types | type coverage | | ||
| | Reduce the time of a slow suite | parallel, profiling | | ||
| | Run one test while you debug | filtering, `--bail`, `--dirty` | | ||
|
|
||
| ## The Assertions of Laravel | ||
|
|
||
| Laravel provides assertions for each part of the framework. Fetch `https://laravel.com/framework/docs/testing` for the complete list, and search for an assertion before building a check by hand. Examples include `assertDatabaseHas()`, `assertModelExists()`, `assertSoftDeleted()`, response assertions such as `assertRedirectToRoute()` and `assertJsonPath()`, and fake assertions such as `Queue::assertPushed()` and `Notification::assertSentTo()`. | ||
|
|
||
| A hand-built check fails with `false is not true`, which identifies nothing. A framework assertion names the incorrect table, value, or response, so the failure indicates what to fix. | ||
|
|
||
| ```php | ||
| // The failure says that false is not true. | ||
| // Instead of this | ||
| expect(User::where('email', 'taylor@laravel.com')->exists())->toBeTrue(); | ||
|
|
||
| // Use this | ||
| // The failure names the table and the attributes that it did not find. | ||
| $this->assertDatabaseHas('users', ['email' => 'taylor@laravel.com']); | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # Fakes, Mocks, and Determinism | ||
|
|
||
| Tests that depend on actual time, randomness, sleeping, or network calls can fail for reasons unrelated to the code under test. Control all four. | ||
|
|
||
| ## How to Isolate a Dependency | ||
|
|
||
| Fetch `https://laravel.com/framework/docs/mocking` for Laravel's fakes, facade doubles, and fake assertions. Confirm each name before using it. | ||
|
|
||
| Identify the dependency, then choose the first applicable option. A framework fake preserves the real code path, while a mock replaces the dependency. | ||
|
|
||
| 1. Use framework fakes for facades such as events, queues, mail, notifications, storage, the HTTP client, time, and sleep. | ||
| 2. Use the fake implementation of the project for a service of the project, if such a fake exists. | ||
| 3. Use a mock for a container-resolved contract only when the real implementation leaves the process or is nondeterministic. | ||
| 4. Use the real implementation for everything else, including the database. | ||
|
|
||
| ## The Fakes | ||
|
|
||
| - Create each fake inside the test that needs it. Do not create fakes in a file-level `beforeEach()`. | ||
| - Pass class names to `Event::fake()` and `Queue::fake()` when you know which classes the code dispatches. A fake without class names can hide an unexpected dispatch. | ||
| - Use a fake without class names only when the test asserts the complete result, including a call to `assertNothingPushed()`. | ||
| - Write one assertion for each fake. The assertion states that the code dispatches the item, or that the code does not dispatch the item. | ||
| - Assert the data of a job or of an event if that data is part of the behavior. | ||
| - Use `Exceptions::fake()` to assert that the application reports the correct exception. Do not use `withoutExceptionHandling()`, because it changes the response under test. | ||
|
|
||
| Create prerequisite factory records before calling `Event::fake()`. Factories use model events, such as a `creating` hook that generates a UUID, and a fake without class names suppresses those events and can produce an invalid model. Call the fake first only when a factory event is under test, and pass that event's class name. | ||
|
|
||
| ## The Mocks | ||
|
|
||
| Use `shouldReceive()` before the action to declare an expectation. Use `shouldHaveReceived()` after the action for a spy. Use `Mockery::on()` or `withArgs()` if an equality check cannot state the expected argument, such as a check of one field of a value object. | ||
|
|
||
| Import the mock function before you use it: `use function Pest\Laravel\mock;`. | ||
|
|
||
| ## The Outbound HTTP | ||
|
|
||
| Call `Http::preventStrayRequests()`. Any request without a matching fake then fails without reaching the network. | ||
|
|
||
| Fake the exact endpoint used by each test. Do not call `Http::fake()` without an endpoint because it accepts unexpected requests and can hide defects. | ||
|
|
||
| ## The Time and the Randomness | ||
|
|
||
| - Freeze the time or move the time in each test that depends on a date, a period, or a timestamp. | ||
| - Use the framework helpers `freezeTime()`, `travelTo()`, `travel()`, and `travelBack()`. Do not call `Carbon::setTestNow()`. | ||
| - Use `Str::createRandomStringsUsing()` to fix a generated string, if the test asserts an identifier or a slug. | ||
| - Use `Sleep::fake()` instead of a real sleep, and assert the sleeps that the code requests. | ||
| - Restore the time and the randomness after each test, if the suite does not restore them for every test. | ||
|
|
||
| ## The Database | ||
|
|
||
| - Run the real query against the real records in the test database. Do not mock the query builder, because the test then asserts the mock. | ||
| - Assert the exact keys of `toArray()` if the shape of the serialized model is a contract. The test then fails when the model exposes a new attribute. | ||
| - Test application behavior caused by the schema, such as deleting dependent records through a cascade. Do not test the database engine's cascade implementation. | ||
| - Use `LazilyRefreshDatabase` instead of `RefreshDatabase`. A test that does not use the database then does not run the migrations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # Naming and Structure | ||
|
|
||
| ## File Layout | ||
|
|
||
| - Name each test file `{ClassName}Test.php`. | ||
| - Place each test file at the same relative path as the class under test. The class `app/Actions/DeleteTeam.php` gets the test `tests/Unit/Actions/DeleteTeamTest.php`. | ||
| - Follow the project's convention for fixture files. If none exists, put fixtures in `tests/Fixtures/` and load them by path. | ||
| - Move large literal values out of the test body and into fixture files. | ||
|
|
||
| ## The Test Function | ||
|
|
||
| Use the test function used by other files in the same directory. If no neighboring test files exist: | ||
|
|
||
| - Use `it()` for the behavior of the code, and write the name as a verb phrase. | ||
| - Use `test()` for a declarative fact, such as a grant in a policy, the labels of an enum, or the shape of a serialized model. | ||
|
|
||
| Use one Pest declaration style in each file. Use either `it()` or `test()` consistently. | ||
|
|
||
| ## The Names of the Tests | ||
|
|
||
| The name of a test is a specification. State the user-visible result and the condition that causes it. | ||
|
|
||
| - Name the behavior, and not the method under test. The file name already gives the class. | ||
| - Give the exact status code in the name of a test for an API error. | ||
| - Do not write `Given`, `When`, or `Then` in the name. | ||
|
|
||
| ```php | ||
| it('returns 401 when no token is provided', function () { ... }); | ||
| it('does not include deployments from deleted environments', function () { ... }); | ||
| it('falls back to the default region when none is configured', function () { ... }); | ||
| ``` | ||
|
|
||
| Use a verb that describes a result, such as `returns`, `renders`, `creates`, `dispatches`, `rejects`, `forbids`, `falls back`, or `does not`. | ||
|
|
||
| Do not write `it('works correctly')` or `it('returns data')`, because neither specifies a meaningful result. Do not write `it('handleMethod creates record')`, because it names a method rather than behavior. | ||
|
|
||
| ## Grouping | ||
|
|
||
| Use `describe()` if one file covers separate actions in a lifecycle. An example is a controller with the actions `index`, `show`, `store`, `update`, and `destroy`. | ||
|
|
||
| Do not use `describe()` in these cases: | ||
|
|
||
| - The file covers one action or one flow. | ||
| - The tests are different only in the input value. Use a dataset instead. | ||
| - The group adds a level but does not make the file easier to read. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an endpoint returns an error response whose body still contains the expected text,
assertSee()passes because it only searches the response content; it does not verify a 200 status or prove that the page rendered successfully. Following this instruction therefore leaves HTTP tests unable to detect status regressions, so successful-page tests should retainassertOk()orassertSuccessful()alongside the content assertion.Useful? React with 👍 / 👎.