Improve consistency of scrolled's Jest test suite and document rules

entry_types/scrolled/doc/internal/testing_conventions.md

@@ -0,0 +1,34 @@
+# Testing Conventions
+
+Conventions for writing JavaScript tests in the `pageflow-scrolled`
+package. Run the suite with `yarn test` and the linter with `yarn lint .`
+from `entry_types/scrolled/package`.
+
+This page defines the shared vocabulary; the topic guides below cover
+each area in depth — read the one matching your task.
+
+## Terminology
+
+- **Kind** — the render approach: a *scoped fixture* (white-box) sets up
+  a subset of providers and renders a component in isolation; an
+  *end-to-end* helper (black-box) renders the whole `Entry` and drives it
+  through page objects.
+- **Scope** — how much of the running app a render helper sets up, from a
+  single provider band (e.g. the content-element scope) up to the full
+  entry. Pick the *smallest scope* sufficient for the behavior.
+- **Context** — the extension environment a spec runs in: plain frontend,
+  inline editing, or commenting. The directory path encodes it
+  (`spec/frontend/`, `spec/frontend/inlineEditing/`,
+  `spec/frontend/commenting/`).
+- **Unit** — the source file, named export, or component under test; the
+  spec path mirrors its source path.
+
+## Guides
+
+- [Render helpers](testing_conventions/render-helpers.md) — the two kinds
+  of helper, how to pick one, and the `useXxx` setup hooks.
+- [Matchers](testing_conventions/matchers.md) — asserting element state;
+  public vs. internal matchers, the polymorphic subject, and how to add
+  one.
+- [Spec file layout](testing_conventions/spec-layout.md) — where a spec
+  file goes; unit specs vs. `features/`; placement across contexts.

entry_types/scrolled/doc/internal/testing_conventions/matchers.md

@@ -0,0 +1,78 @@
+# Matchers
+
+Part of the [Testing Conventions](../testing_conventions.md) guide, which
+defines the **kind / scope / context / unit** vocabulary used here.
+
+Custom matchers assert the visual/structural state of a single element.
+They keep page objects focused on *locating* and *acting*, and let the
+same assertion run regardless of which render helper produced the
+element.
+
+## Polymorphic subject
+
+Every matcher resolves its subject through `getElement(subject)`
+(`src/testHelpers/matchers/getElement.js`), which returns
+`subject?.el ?? subject`. The subject can therefore be either:
+
+- a **DOM element** — e.g. the `container` from `renderInContentElement`, or
+- a **page object** — e.g. `getContentElementByTestId(...)` from
+  `renderEntry` (page objects expose their anchor as `.el`).
+
+```js
+expect(container).toContainContentElementBox({borderRadius: 'circle'});
+expect(getContentElementByTestId('probe')).toHaveAlignment('right');
+```
+
+## Public vs internal
+
+The dividing line is **what the content element controls** vs **what the
+framework applies around it**. Each tier below has a fixed subject and
+setup hook; the matchers live one file per matcher in the directory
+noted, which is the source of truth for their exact signatures and
+assertions.
+
+**Public matchers** cover chrome a content element opts into through
+framework components — plugin authors need these to test their own
+components. Shipped via `pageflow-scrolled/testHelpers`, enabled with
+`useContentElementMatchers()`; the subject is a `renderInContentElement`
+container or a `renderEntry` page object. In `src/testHelpers/matchers/`:
+`toContainContentElementBox`, `toContainFitViewport`.
+
+**Internal layout matchers** cover framework state applied *around* the
+element (margins, scroll space, alignment). They depend on the
+entry-level layout, so the subject must come from `renderEntry` —
+`renderInContentElement` does not render these wrappers. Enabled with
+`useContentElementLayoutMatchers()`. In
+`spec/support/matchers/contentElement/`: `toHaveContentElementMargin`,
+`toHaveScrollSpace`, `toHaveAlignment`.
+
+**Section matchers** assert a section's foreground/layout state. The
+subject is a section page object (`getSectionByPermaId(...)`), so they
+also require `renderEntry`. Enabled with `useSectionMatchers()`. In
+`spec/support/matchers/section/`: `toHaveSuppressedPadding`,
+`toHaveRemainingSpace`, `toHaveForcedPadding`, `toHaveFadedOutForeground`,
+`toHavePerElementFadeTransition`, `toHaveFirstBoxSuppressedTopMargin`,
+`toHaveConstrainedContentWidth`.
+
+The `useXxx` hooks are listed under
+[Setup hooks](render-helpers.md#setup-hooks).
+
+## Adding a matcher
+
+Ask: *would an external plugin's test suite need this to verify their
+component behaves correctly?* If yes, it is public; if it asserts
+framework state the plugin does not control, it is internal. Register it
+through the hook for its tier — `useContentElementMatchers` (public),
+`useContentElementLayoutMatchers` (internal layout), or
+`useSectionMatchers` (section) — so specs opt in explicitly, mirroring
+`usePageObjects`.
+
+## Checks
+
+- ❌ An internal layout or section matcher asserted on a subject from
+  `renderInContentElement` — those wrappers only exist under `renderEntry`.
+- ❌ A new matcher registered in the wrong tier — public is only for
+  chrome a plugin controls through framework components; framework state
+  applied around the element is internal.
+- ❌ A matcher's job duplicated in a page object — matchers assert; page
+  objects locate and act.

entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md

@@ -0,0 +1,82 @@
+# Render helpers
+
+Part of the [Testing Conventions](../testing_conventions.md) guide, which
+defines the **kind / scope / context / unit** vocabulary used here.
+
+## Two kinds of render helpers
+
+**Scoped fixtures** (white-box) set up the providers/state for a single
+scope and render a component in isolation of the surrounding entry. Most
+ship as part of the public API (`pageflow-scrolled/testHelpers`) so that
+content element plugins can use them too.
+
+**End-to-end helpers** (black-box) render the whole entry through the
+official `Entry` component and interact with it via *page objects*. They
+live in `spec/support/` and are internal to this package.
+
+The import path follows from the kind: public scoped fixtures from
+`pageflow-scrolled/testHelpers`; page objects and `renderEntry` from
+`support/pageObjects`.
+
+| Helper | Scope |
+| --- | --- |
+| `renderInEntry` | Entry state + `RootProviders`. For any component that needs entry state but nothing more. |
+| `renderHookInEntry` | Same as `renderInEntry`, for selector hooks that read entry state. |
+| `renderInContentElement` | Content-element scope: attributes, lifecycle, command emitter, optional inline-editing context. For components rendered *inside* a content element. |
+| `renderEntry` | Full `Entry`, driven through page objects (`getContentElementByTestId`, `getSectionByPermaId`). For cross-cutting frontend features and integration behavior. |
+
+`renderWithReviewState` is an internal scoped fixture for `src/review/`
+UI, imported from `support/renderWithReviewState`; it sets up
+`ReviewStateProvider` only.
+
+## Picking a helper
+
+Pick the smallest-scope helper sufficient for the behavior under test. A
+component's scope is set by what it must render against — which providers
+and state:
+
+- A content element's component (`src/contentElements/<name>/<Component>.js`)
+  → `renderInContentElement`, asserting with [matchers](matchers.md).
+- A reusable frontend component (`src/frontend/<Component>.js`) →
+  `renderInEntry`, or `renderInContentElement` if it needs the
+  content-element scope.
+- A cross-cutting feature that only exists *because pieces compose*
+  (margins between elements, section transitions, alignment within a
+  layout) → `renderEntry` with page objects.
+
+`renderInContentElement` returns extra controls beyond the
+`@testing-library/react` result — `simulateScrollPosition`,
+`triggerEditorCommand`, `simulateStorylineMode` — and takes an
+`inlineEditing` option that opts the element into an inline-editing
+context (pass `true` for editable defaults, or an object to override
+individual flags). See the helper's source for the exact surface.
+
+## Setup hooks
+
+Setup hooks are `useXxx()` functions called at the top of a `describe`
+block; each installs a `beforeEach`. Group them at the top of the block,
+in the order they appear below.
+
+| Hook | Import from | Installs |
+| --- | --- | --- |
+| `usePageObjects` | `support/pageObjects` | Page-object queries for `renderEntry`, the `withTestId` helper content element, and `jest.restoreAllMocks()` per test. |
+| `useInlineEditingPageObjects` / `useCommentingPageObjects` | `support/pageObjects` | Page-object sugar for the respective extension, built on `usePageObjects`. |
+| `useContentElementMatchers` | `pageflow-scrolled/testHelpers` | The public content element [matchers](matchers.md). |
+| `useContentElementLayoutMatchers` | `support/matchers` | The internal content element layout [matchers](matchers.md). |
+| `useSectionMatchers` | `support/matchers` | The internal section [matchers](matchers.md). |
+| `useFakeFeatures` | `pageflow/testHelpers` | Enables named feature flags for the test. |
+
+(`useEditorGlobals`, `useFakeMedia`, and `useFakeParentWindow` are
+further fixtures for editor, media, and parent-window scenarios.)
+
+## Checks
+
+- ❌ Reaching for `renderEntry` where a scoped fixture suffices — use the
+  smallest scope the behavior needs; `renderEntry` is for behavior that
+  only exists because pieces compose.
+- ❌ Asserting an internal layout or section [matcher](matchers.md) on a
+  subject from `renderInContentElement` — those wrappers only exist under
+  `renderEntry`.
+- ❌ The same custom `wrapper` copied across specs — a recurring setup
+  need signals the testing API should grow a named option (like
+  `renderInContentElement`'s `inlineEditing`), not repeated wrappers.

entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md

@@ -0,0 +1,109 @@
+# Spec file layout
+
+Part of the [Testing Conventions](../testing_conventions.md) guide, which
+defines the **kind / scope / context / unit** vocabulary used here.
+
+Specs mirror source structure. This guide tells you where a new spec file
+goes: start with the decision procedure; the rationale and checks below
+settle the cases it doesn't spell out.
+
+## Where does my spec go?
+
+Find the source you're testing, then read off the spec path:
+
+| Source | Spec path |
+| --- | --- |
+| `src/Foo.js` (unsplit) | `spec/Foo-spec.js` |
+| `src/Foo.js` or `src/Foo/index.js`, split into topics | `spec/Foo/features/<topic>-spec.js` |
+| `src/Foo/<helper>.js` | `spec/Foo/<helper>-spec.js` |
+| a **named export** of `src/Foo/index.js` | `spec/Foo/<export>-spec.js` |
+
+Two kinds of spec live under a `spec/Foo/` directory, and they answer to
+different sources:
+
+- **Unit specs** mirror a single source unit — a `src/Foo/<helper>.js`
+  file or a named export of the main file — and sit at the directory
+  level, named after that unit. Example: `AudioPlayer/index.js`
+  exports both the `AudioPlayer` component and a `processSources`
+  helper, so `processSources-spec.js` tests that helper at the
+  directory level.
+- **Topic splits** of the main unit's own behavior go under `features/`,
+  never beside it. The `AudioPlayer` component's rendered behavior (e.g.
+  its structured-data output) is a topic, so it lives at
+  `features/structuredData-spec.js`.
+
+`features/` is the *only* place topic splits live — for a `.js` file and
+a `/` directory unit alike. These specs are written against the unit's
+stable public interface, so they tend to outlive the internal helpers
+they exercise.
+
+## Placement across contexts
+
+The directory path encodes the *context* a spec runs in:
+
+| Directory | Context (loaded extension) |
+| --- | --- |
+| `spec/frontend/` | none |
+| `spec/frontend/inlineEditing/` | inline editing |
+| `spec/frontend/commenting/` | commenting |
+
+When a unit behaves differently across contexts, give it **one spec per
+context**, named after the unit, in the matching directory. The path
+conveys the context, so the filenames stay identical.
+
+The context also determine which render helper and setup hooks a spec
+uses — see [render helpers](render-helpers.md).
+
+## Which level owns `features/`?
+
+`features/` groups a unit's behavior topics, so it sits at the level of
+whatever it tests. Ask whether that behavior has a single source
+counterpart:
+
+- **It does** → `features/` nests under that unit. The `EditableText`
+  component (`src/frontend/inlineEditing/EditableText/`) owns
+  `spec/frontend/inlineEditing/EditableText/features/`, with its helper
+  specs (`blocks-spec.js`, `marks-spec.js`, …) alongside.
+- **It doesn't** — the behavior *is* a context's integration
+  (`contentElementSelection`, `marginIndicator` for inline editing) →
+  `features/` sits at that context root,
+  `spec/frontend/inlineEditing/features/`.
+
+## Rationale
+
+- **Why `features/` is the only home for topic splits.** It keeps spec
+  layout invariant under a `src/Foo.js → src/Foo/index.js` refactoring:
+  the topic specs don't move when the source file becomes a directory.
+- **Why a unit's behavior tests live in exactly one place.** A flat
+  `spec/Foo-spec.js` and a `spec/Foo/features/` directory both holding
+  behavior tests is the tell-tale of that refactoring leaving the
+  top-level spec behind. They must not coexist (see checks).
+- **Why the render helper doesn't decide ownership.** The helper a spec
+  uses is the *mechanism*, not the unit. Most feature specs use
+  `renderEntry`, but that never turns a component into a topic of its
+  parent context — the source counterpart at the path does.
+- **Why prefer the public interface over its wiring.** When behavior
+  changes across contexts because an extension swaps a provider — an
+  implementation detail you may want to refactor — test it end-to-end
+  through a fake consumer with `renderEntry`, not by white-box-driving
+  the replacement mechanism (which couples the spec to how providers are
+  registered, not to the contract).
+
+## Checks
+
+Scan for these before opening a PR:
+
+- ❌ `spec/Foo-spec.js` coexisting with `spec/Foo/features/` — a
+  refactoring left the top-level spec behind; fold it into `features/`.
+  (Helper and named-export unit specs at `spec/Foo/<helper>-spec.js`
+  *may* sit beside a flat `spec/Foo-spec.js` — they test different units.
+  It is specifically `Foo-spec.js` together with `Foo/features/` that
+  must not coexist.)
+- ❌ A topic split living outside `features/` (e.g. beside the helper
+  specs) — move it under `features/`.
+- ❌ A `features/` directory floated up to the context root when the
+  behavior *does* have a source counterpart deeper in the tree — push it
+  down to the unit that owns it.
+- ❌ The same spec duplicated per context when its behavior doesn't
+  actually differ by context — one spec per context is for behavior that
+  *changes* across them, not identical assertions.

entry_types/scrolled/package/documentation.yml

@@ -53,14 +53,18 @@ toc:
       Helper functions that can be used in content elements.
     children:
       - paletteColor
-  - name: Spec Support
+  - name: Test Helpers
     description: |
-      Helper functions to use in specs.
+      Helpers for testing content elements, available via
+      `pageflow-scrolled/testHelpers`.
     children:
       - normalizeSeed
       - renderInEntry
       - renderInContentElement
       - renderHookInEntry
+      - useContentElementMatchers
+      - toContainContentElementBox
+      - toContainFitViewport
   - name: Storybook Support
     description: |
       Helper functions to use in content element stories.