mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 02:19:55 +08:00
RSpec setup becomes harder to follow at either extreme: trivial fixture wrappers hide lifecycle and intent, while forcing every named operation inline repeats low-level protocol and configuration details. This change documents and applies a test-setup hierarchy: - use `fab!`, `let`, `let!`, `subject`, and inline `Fabricate` according to lifecycle and role; - use a small example-group method when parameterized behavior gives one spec useful vocabulary; - move helpers into auto-loaded `spec/support` only when they are shared across spec files; - use fabricators and page objects for the data shapes and system-test interfaces they own. Core and plugin support files are loaded centrally by `rails_helper`, so plugin-specific support loaders are unnecessary. The migration specs encountered during the sweep are removed according to repository policy; production migrations are unchanged.
7 KiB
Vendored
7 KiB
Vendored
| name | description |
|---|---|
| discourse-writing-rspec-tests | Write and structure RSpec tests for Discourse core, plugins, themes, or theme components. Use when creating or modifying model specs, controller specs, service specs, job specs, system tests, or integration tests. Covers fabricators, page objects, test structure, and theme test setup. |
Writing RSpec Tests
Discourse uses RSpec for testing. Follow these patterns for all test types.
Testing Principles
- Test behavior, not implementation — test public interfaces; don't assert on internal state or private methods. Refactoring internals shouldn't break tests.
- Choose assertions at the public boundary — for a query, assert its return value. For a command, drive its public entry point and assert the direct side effect it owns: persisted state, an enqueued job, an emitted event, a response body, or rendered output. Group class, service, job, and model specs by that entry point (
describe ".call",describe "#execute", ordescribe "#expire!"). - One concept per test — each
itblock verifies one behavior for clear failure diagnosis. - Don't over-mock — mock external boundaries (HTTP, third-party services), not internal collaborators. Too many mocks signals a design problem.
- Don't assert that internal methods are or aren't called — assertions like
SomeService.expects(:some_method).never(or.once,.with(...)) couple the test to internal implementation details that the caller shouldn't care about. Assert on the observable outcome instead: returned value, persisted state, emitted event, response body, rendered output. If the implementation is later refactored, inlined, or renamed, a behavior-focused test still passes when the behavior is correct. - Capture infrastructure side effects without mocking internals — use
DiscourseEvent.track_eventsandMessageBus.track_publishwhen asserting emitted events or published messages instead of expecting calls totriggerorpublish. - Prefer readability over DRYness — tests are documentation. Some duplication is fine. Avoid deep
shared_examples/letchains that hurt readability. - Choose the smallest fitting test primitive — use
fab!for records shared across examples,letfor lazy per-example values,let!when a per-example record must exist before the action, andsubjectfor the operation under test. Use inlineFabricatefor a record local to one example. Becausefab!uses TestProflet_it_be, it cannot depend onletor other per-example setup. Keepletvalue-oriented rather than hiding a parameterized helper in a Proc or lambda. When parameterized behavior needs a name to keep one spec readable, define a small method in that example group; do not introduce one merely to wrap a simpleFabricate,create!, or direct call. Move the method into a focused, auto-loadedspec/supportmodule only when it is shared by multiple spec files. Searchspec/fabricatorsbefore creating setup by hand, and define a derived fabricator for a recurring record shape. - Test edge cases — nil inputs, empty collections, boundary values, permission failures — not just happy paths.
- Keep tests independent — no test should depend on another test's execution or shared mutable state.
- Verify placement in parent context — before adding a new test, always read the surrounding
describe/contextblock to confirm the test belongs there. Check that the parent context's description,let/fab!setup, andbeforehooks match the scenario being tested. A misplaced test inherits the wrong setup and produces misleading results. - Arrange-Act-Assert — clear separation of setup, action, and verification in each test.
- Don't test framework behavior — don't test that Rails validations work; test your business logic.
- Each layer asserts what it owns — models own validations, scopes, callbacks, and persisted state; services own orchestration, authorization, and return values; request specs own the HTTP contract and externally visible effects of the action; jobs own execution behavior and any required idempotency. Don't re-assert a lower layer's contract from above. Test callbacks through the public operation that invokes them, not with a group such as
describe "after_commit :callback_name". - No single-letter block variables — use descriptive names like
|vote|,|option|, not|v|,|o|. - Assert collections in a single assertion — use
contain_exactlyoreqinstead of multipleinclude/not_to includechecks. - Reference objects, not literal strings, in negative assertions —
expect(response.body).not_to include("hidden data explorer excerpt")silently passes if the literal has a typo or drifts from the source, giving a false sense of security. Reference the object directly (expect(response.body).not_to include(private_post.raw)) so the assertion stays in sync with the data under test. The same applies to anynot_to include/not_to matchagainst hardcoded strings. - Optimise for human readability — minimise context overload when reading an example. Avoid too many indirections.
- Limit nesting to 2 levels — avoid more than 2 levels of
describe/contextnesting. Instead of deeply nested contexts, put the full scenario description in theitblock itself. Flat tests are easier to read and maintain. - Avoid double negatives in descriptions — write test descriptions that state the positive condition. For example, prefer
"returns true when topic_approval_type is approval or pre_approval"over"returns true when topic_approval_type is not none". Be specific about the values being tested.
Test Efficiency
Tests have setup overhead. Optimize for the fewest test examples possible:
- Combine related assertions in a single
itblock when testing the same page/state - Avoid separate tests for trivial variations
- Each
itblock incurs setup overhead; batch checks where logical - Use one system test per user flow, not per internal code path. If scenarios look the same to the user but differ internally, test the flow once and cover the branches with cheaper tests.
Running Tests
# Run specific file
bin/rspec spec/models/topic_spec.rb
# Run specific line
bin/rspec spec/models/topic_spec.rb:15
Specialized Test Types
- Request specs: See references/request-specs.md for controller/request spec structure, action-based
describegrouping, and what to assert. - System tests: See references/system-tests.md for file naming, test structure, page objects, and scoping patterns.
- Theme/component tests: See references/theme-tests.md for theme upload helpers, settings, and directory structure.
Tracking Helpers
See references/tracking-helpers.md for DiscourseEvent.track_events, MessageBus.track_publish, and track_sql_queries — block helpers that capture events, message-bus publishes, and SQL queries so tests can assert on side effects.