mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 02:51:10 +08:00
This commit expands the system-tests skill reference: a new "Test Selectors in Templates" section documents that `data-test-*` attributes get stripped from production builds by ember-test-selectors, a new "Assertors Must Use Capybara Matchers" section forbids `find(...).text == ...` patterns in page object assertors (they bypass Capybara's auto-retry and flake on re-renders), a new "Debugging Failed System Tests" section walks through diagnosing failures with strategic `puts` / `console.log` logging and lists common failure patterns, and the existing "Writing `it` Block Descriptions" section is reworked to consistently frame descriptions from the user's perspective.
397 lines
15 KiB
Markdown
Vendored
397 lines
15 KiB
Markdown
Vendored
# System Tests
|
|
|
|
Discourse uses Rails system tests with RSpec and Capybara. Tests run both server and client code together.
|
|
|
|
## Assert What Users See, Not Database State
|
|
|
|
System tests simulate real user interactions. **Assert on visible UI state, not database records.** If a user can't see it on the page, don't check it in the database.
|
|
|
|
```rb
|
|
# Good - asserts what the user sees
|
|
expect(topic_page).to have_post(text: "Hello world")
|
|
expect(toasts).to have_success("Topic created")
|
|
|
|
# Bad - checks database state the user can't see
|
|
expect(Topic.last.title).to eq("Hello world")
|
|
expect(Post.count).to eq(1)
|
|
```
|
|
|
|
Only reach into the database when no UI signal exists for the behavior being tested, which should be rare. If you find yourself needing database assertions, consider whether the feature is missing user-visible feedback.
|
|
|
|
**Verify persistence by refreshing the page.** After a save action, refresh (or revisit) the current page and assert the saved state is still visible. This is what a real user would do to confirm their changes persisted.
|
|
|
|
```rb
|
|
# Good - refreshes and verifies saved state
|
|
admin_settings_page.save
|
|
expect(toasts).to have_success("Settings saved")
|
|
|
|
page.refresh
|
|
|
|
expect(admin_settings_page).to have_setting_value("site_name", "My Forum")
|
|
|
|
# Bad - only checks before refresh, or checks the database
|
|
admin_settings_page.save
|
|
expect(SiteSetting.site_name).to eq("My Forum")
|
|
```
|
|
|
|
## File Naming
|
|
|
|
Use `action_scenario_spec.rb` pattern - describe what the test does:
|
|
- `filter_sidebar_spec.rb` → `RSpec.describe "Filter sidebar"`
|
|
- `toggle_dark_mode_spec.rb` → `RSpec.describe "Toggle dark mode"`
|
|
- `create_topic_with_template_spec.rb` → `RSpec.describe "Create topic with template"`
|
|
|
|
Avoid generic names like `feature_spec.rb` or `my_feature_spec.rb`.
|
|
|
|
## Test Structure
|
|
|
|
```rb
|
|
# frozen_string_literal: true
|
|
|
|
RSpec.describe "Filter sidebar" do
|
|
fab!(:user)
|
|
fab!(:category)
|
|
|
|
let(:sidebar) { PageObjects::Components::Sidebar.new }
|
|
|
|
context "when logged in" do
|
|
before { sign_in(user) }
|
|
|
|
it "filters and displays categories" do
|
|
visit("/")
|
|
|
|
sidebar.filter(category.name)
|
|
|
|
expect(sidebar).to have_category(category)
|
|
expect(sidebar).to have_no_category("Unrelated")
|
|
end
|
|
end
|
|
end
|
|
```
|
|
|
|
**Key patterns:**
|
|
- Instantiate page objects with `let`
|
|
- Use `context` blocks to group related scenarios
|
|
- Batch related assertions in a single `it` block
|
|
- Never use raw selectors in test files - delegate to page objects
|
|
|
|
## Writing `it` Block Descriptions
|
|
|
|
System tests exercise the product from the user's seat. Descriptions should read as **what the user does, sees, or experiences when interacting with the system** — never as what the system internally does in response.
|
|
|
|
A good check: if you removed the `it`, the description should still sound like a sentence a user (or a PM writing acceptance criteria) would say. Phrases like "displays...", "updates...", "sets...", "renders...", "calls...", "persists..." are the system's voice — rewrite them from the user's side.
|
|
|
|
```rb
|
|
# Good - user-perspective: what the user does or sees
|
|
it "lets the user filter categories in the sidebar"
|
|
it "shows the user an error when they submit an empty title"
|
|
it "takes the user to the topic after they click a search result"
|
|
it "keeps the user's tag filter visible after they switch categories"
|
|
|
|
# Bad - system-perspective: what the system does internally
|
|
it "displays an error when title is blank"
|
|
it "navigates to the topic on search result click"
|
|
it "preserves the tag filter in the URL when switching categories"
|
|
it "updates the model count after deletion"
|
|
it "sets the correct query param on filter change"
|
|
```
|
|
|
|
## Freezing Time
|
|
|
|
For time-sensitive system tests, use the `time:` metadata key instead of calling `freeze_time` manually. This freezes **both** the Ruby server time and the Playwright browser clock simultaneously:
|
|
|
|
```rb
|
|
it "shows the post timestamp correctly", time: Time.zone.parse("2024-01-15 10:00:00") do
|
|
visit(topic_path(topic))
|
|
expect(post_component).to have_timestamp("Jan 15")
|
|
end
|
|
```
|
|
|
|
**Why this matters:** In system tests, JavaScript also needs to see the frozen time (e.g. for relative timestamps like "5 minutes ago"). The `time:` metadata calls both `freeze_time` and `pw_page.clock.set_fixed_time` under the hood. Using plain `freeze_time` only freezes the Ruby side, leaving the browser clock running normally.
|
|
|
|
## Common Helpers
|
|
|
|
| Helper | Purpose |
|
|
|--------|---------|
|
|
| `sign_in(user)` | Sign in as a user |
|
|
| `visit(path)` | Navigate to a page |
|
|
| `puts` | Debug server-side code (Ruby) |
|
|
| `console.log` | Debug client-side code (JavaScript) |
|
|
| `pause_test` | Pause to view visual state (use with `SELENIUM_HEADLESS=0`) |
|
|
|
|
## Debugging Failed System Tests
|
|
|
|
**When a system test fails, diagnose before fixing.** Guessing at fixes without understanding the failure burns retries and lands the wrong patch.
|
|
|
|
If you changed frontend code (`.js` / `.hbs` / `.gjs` / `.gts`) and the behavior suggests your changes aren't being picked up, the asset build is stale. Run `bin/ember-cli --build` to rebuild, then re-run the test. This is only needed when `bin/ember-cli` isn't already running in the background.
|
|
|
|
For runtime visibility, add `puts "DEBUG: …"` in Ruby (controllers, models, services, jobs) or `console.log("DEBUG: …")` in JavaScript (components, services, routes). Place logs at the entry point of the code path, around conditional branches, and at the line where the failure occurs. Re-run with documentation format so the output reads cleanly:
|
|
|
|
```sh
|
|
bin/rspec spec/system/some_spec.rb:LINE --format documentation
|
|
```
|
|
|
|
Read the debug output and ask: what values are actually present versus expected? Is the code path reached at all? Is there a timing issue? Is the test data set up correctly? Once you understand the root cause, make a targeted fix and **remove every `DEBUG:` log line** before finalizing.
|
|
|
|
Common failure patterns:
|
|
|
|
| Symptom | Likely cause | Debug approach |
|
|
|---|---|---|
|
|
| Element not found | Selector wrong, element not rendered, timing | `console.log` in the component, double-check the selector in the test |
|
|
| Unexpected content | Wrong data, rendering issue | `puts` in the controller/serializer to check data flow |
|
|
| JS changes not reflected | Assets not rebuilt | Run `bin/ember-cli --build` |
|
|
| Flaky pass/fail | Timing issue | Add waits, check for async operations |
|
|
| 404/500 in test | Route or controller issue | `puts` in the route handler, check server logs |
|
|
|
|
## Test Selectors in Templates
|
|
|
|
Discourse's Ember app ships [`ember-test-selectors`](https://github.com/mainmatter/ember-test-selectors), which **strips every `data-test-*` attribute from production builds** at compile time. They're free to add in `.gjs` / `.hbs` templates: tests and dev builds see them, end users never do.
|
|
|
|
Reach for `data-test-*` when no stable, semantic selector exists — the element has no meaningful class, no ARIA role, no text content you'd want to assert on, or the existing classes are tied to styling and could change. Don't sprinkle them on elements that already have a good selector; prefer asserting against user-visible structure when one exists.
|
|
|
|
```hbs
|
|
{{! Good - opaque element needs a test hook }}
|
|
<div data-test-empty-state-title>{{@title}}</div>
|
|
<div data-test-empty-state-body>{{@body}}</div>
|
|
```
|
|
|
|
Page objects then reference them like any other selector:
|
|
|
|
```rb
|
|
def has_title?(text)
|
|
has_css?("[data-test-empty-state-title]", text: text)
|
|
end
|
|
```
|
|
|
|
Because the addon strips these in production, never use `data-test-*` for styling, JS behavior, or anything outside tests — those will disappear when users load the site.
|
|
|
|
## Page Objects
|
|
|
|
**Always use page objects** - never write raw selectors or Capybara finders directly in test files. Encapsulate all selectors and interactions in page object classes. Only add methods that are actually used by your tests - don't add methods speculatively.
|
|
|
|
**Prefer passing entire objects** to page object methods instead of just attributes. Let the page object extract what it needs:
|
|
|
|
```rb
|
|
# Good - page object receives the full object
|
|
def has_tag?(tag)
|
|
has_css?("#{SELECTOR} .discourse-tag", text: tag.name)
|
|
end
|
|
|
|
# In test
|
|
expect(component).to have_tag(tag)
|
|
```
|
|
|
|
```rb
|
|
# Avoid - caller extracts the attribute
|
|
def has_tag?(name)
|
|
has_css?("#{SELECTOR} .discourse-tag", text: name)
|
|
end
|
|
|
|
# In test
|
|
expect(component).to have_tag(tag.name)
|
|
```
|
|
|
|
**Combine related assertions** into single page object methods when checking multiple aspects of the same element:
|
|
|
|
```rb
|
|
# Good - checks both content and link in one assertion
|
|
def has_category_link?(category)
|
|
has_css?("a.category-link[href='/c/#{category.slug}']", text: category.name)
|
|
end
|
|
|
|
# In test - single assertion covers both aspects
|
|
expect(sidebar).to have_category_link(category)
|
|
```
|
|
|
|
```rb
|
|
# Avoid - separate methods when they could be combined
|
|
def has_category_name?(category)
|
|
has_css?(".category-name", text: category.name)
|
|
end
|
|
|
|
def has_category_link?(category)
|
|
has_css?("a[href='/c/#{category.slug}']")
|
|
end
|
|
|
|
# In test - multiple assertions for one element
|
|
expect(sidebar).to have_category_name(category)
|
|
expect(sidebar).to have_category_link(category)
|
|
```
|
|
|
|
## Assertors Must Use Capybara Matchers
|
|
|
|
**Page object assertors (`has_*?` / `have_no_*?` methods) must use Capybara's matchers (`has_css?`, `has_text?`, `has_selector?`, `has_field?`, `has_no_css?`, etc.). Never fetch a reference with `find` / `all` and then apply an equality matcher to it.**
|
|
|
|
Capybara matchers re-query the DOM on every retry up to `Capybara.default_max_wait_time`. Patterns that capture an element first and assert against it after bypass that re-query — the reference goes stale the moment the DOM updates (a re-render, an async response, an animation finishing) and the test flakes with `Selenium::WebDriver::Error::StaleElementReferenceError` or a value mismatch that disappears on rerun.
|
|
|
|
```rb
|
|
# Good - has_css? re-queries until it matches or times out
|
|
def has_username?(user)
|
|
has_css?(".user-card .username", text: user.username)
|
|
end
|
|
|
|
def has_avatar_for?(user)
|
|
has_css?(".user-card img.avatar[src='#{user.avatar_url}']")
|
|
end
|
|
```
|
|
|
|
```rb
|
|
# Bad - reference fetched once, equality applied after the fact
|
|
def has_username?(user)
|
|
find(".user-card .username").text == user.username
|
|
end
|
|
|
|
# Bad - same problem with RSpec equality matchers on a fetched node
|
|
def has_avatar_for?(user)
|
|
expect(find(".user-card img.avatar")[:src]).to eq(user.avatar_url)
|
|
end
|
|
|
|
# Bad - storing the node for later assertion
|
|
let(:username_el) { find(".user-card .username") }
|
|
|
|
it "shows the username" do
|
|
expect(username_el.text).to eq(user.username) # stale the moment the card re-renders
|
|
end
|
|
```
|
|
|
|
The same rule applies to negative assertions. Use `has_no_css?` (or `expect(...).to have_no_css(...)` / `expect(...).not_to have_css(...)`, which RSpec routes through the matcher's waiting form). Never assert on the boolean negation of a positive predicate — `expect(!page.has_css?(...))` returns immediately and does not wait for the element to disappear.
|
|
|
|
```rb
|
|
# Good - waits for the element to be gone
|
|
def has_no_spinner?
|
|
has_no_css?(".loading-spinner")
|
|
end
|
|
|
|
# Bad - returns false the moment the spinner exists, no waiting
|
|
def has_no_spinner?
|
|
!has_css?(".loading-spinner")
|
|
end
|
|
```
|
|
|
|
## Composing Page Objects
|
|
|
|
**Don't stuff everything into a single page object.** Identify UI boundaries (header, sidebar, composer, post, modal, etc.) and split them into separate component page objects. The main page object exposes methods that return instances of those components.
|
|
|
|
```rb
|
|
# Good - main page composes scoped components
|
|
module PageObjects
|
|
module Pages
|
|
class Topic < PageObjects::Pages::Base
|
|
def header
|
|
PageObjects::Components::TopicHeader.new
|
|
end
|
|
|
|
def composer
|
|
PageObjects::Components::Composer.new
|
|
end
|
|
|
|
def post_by_number(number)
|
|
PageObjects::Components::Post.new(".topic-post[data-post-number='#{number}']")
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
# In test
|
|
topic_page.visit_topic(topic)
|
|
topic_page.header.click_reply
|
|
topic_page.composer.fill_in_body("Hello")
|
|
expect(topic_page.post_by_number(2)).to be_liked
|
|
```
|
|
|
|
```rb
|
|
# Avoid - one mega page object with every selector and interaction
|
|
class Topic < PageObjects::Pages::Base
|
|
def click_header_reply; end
|
|
def fill_composer_body(text); end
|
|
def post_2_liked?; end
|
|
# ...dozens more methods spanning unrelated UI regions
|
|
end
|
|
```
|
|
|
|
Component page objects keep selectors local to the region they describe, make tests read like the UI is structured, and let components be reused across pages (e.g. the composer appears in multiple pages).
|
|
|
|
## Scoping Components to Elements
|
|
|
|
When a component appears multiple times on a page (e.g., a like button on each post), **always scope components to their parent element.**
|
|
|
|
**Bad - ambiguous component:**
|
|
```rb
|
|
let(:like_button) { PageObjects::Components::LikeButton.new }
|
|
|
|
it "likes a reply" do
|
|
topic_page.visit_topic(topic)
|
|
like_button.click # Which post's like button?
|
|
expect(like_button).to be_liked # Ambiguous
|
|
end
|
|
```
|
|
|
|
**Good - component scoped through parent:**
|
|
```rb
|
|
let(:topic_page) { PageObjects::Pages::Topic.new }
|
|
|
|
it "likes a reply" do
|
|
topic_page.visit_topic(topic)
|
|
topic_page.post_by_number(2).click_like
|
|
expect(topic_page.post_by_number(2)).to be_liked
|
|
end
|
|
```
|
|
|
|
**Two valid patterns for scoping:**
|
|
|
|
1. **Access through parent page object** (preferred) - The parent provides a method that returns a scoped component:
|
|
```rb
|
|
# In page object
|
|
def post_by_number(number)
|
|
PageObjects::Components::Post.new(".topic-post[data-post-number='#{number}']")
|
|
end
|
|
|
|
# In test
|
|
topic_page.post_by_number(2).click_like
|
|
```
|
|
|
|
2. **Pass scoping element to constructor** - Instantiate with a selector that scopes to the specific instance:
|
|
```rb
|
|
let(:second_post) { PageObjects::Components::Post.new(".topic-post[data-post-number='2']") }
|
|
|
|
it "likes the second post" do
|
|
second_post.click_like
|
|
expect(second_post).to be_liked
|
|
end
|
|
```
|
|
|
|
The first pattern is preferred because it keeps selectors out of test files and makes the relationship explicit.
|
|
|
|
## S3 Upload System Tests
|
|
|
|
System specs involving S3 uploads use MinIO (a local S3-compatible server). The test infrastructure automatically downloads and runs MinIO with an isolated data store during specs.
|
|
|
|
**Writing S3 system specs:** Call `setup_or_skip_s3_system_test` as the very first line in each `it` block. This configures the S3 environment and skips the test if S3 specs aren't enabled:
|
|
|
|
```rb
|
|
it "uploads a file to S3" do
|
|
setup_or_skip_s3_system_test
|
|
|
|
# your test code here
|
|
end
|
|
```
|
|
|
|
**Running S3 system specs:** S3-related specs are skipped by default. Enable them with the `RUN_S3_SYSTEM_SPECS` environment variable:
|
|
|
|
```sh
|
|
RUN_S3_SYSTEM_SPECS=1 bin/rspec spec/system/s3_uploads_spec.rb
|
|
```
|
|
|
|
**Local setup required:** MinIO must be reachable at specific hostnames. Add entries to `/etc/hosts`:
|
|
|
|
```sh
|
|
# Linux
|
|
echo "127.0.0.1 minio.local discoursetest.minio.local" | sudo tee -a /etc/hosts
|
|
|
|
# macOS — also needs IPv6 entries
|
|
echo "127.0.0.1 minio.local discoursetest.minio.local" | sudo tee -a /etc/hosts
|
|
echo "::1 minio.local discoursetest.minio.local" | sudo tee -a /etc/hosts
|
|
echo "fe80::1%lo0 minio.local discoursetest.minio.local" | sudo tee -a /etc/hosts
|
|
```
|
|
|
|
To use a custom MinIO hostname, set `MINIO_RUNNER_MINIO_DOMAIN`.
|