From ae6129e49048add75134b26c6499b06dabbdefc8 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Tue, 9 Jun 2026 22:15:58 +0200 Subject: [PATCH 01/11] Add content element chrome test matchers Expose toContainContentElementBox and toContainFitViewport via pageflow-scrolled/testHelpers so content element plugin authors can assert box and FitViewport state directly, without relying on internal page objects or full entry renders. The matchers accept a plain DOM node or a page object and run under the lightweight renderInContentElement helper. Migrate the inline image and content element box specs as the first adopters and drop the page object predicates they replace. --- .../inlineImage/InlineImage-spec.js | 109 +++++++----------- .../features/contentElementBox-spec.js | 19 +-- .../package/spec/support/pageObjects/index.js | 79 +------------ .../scrolled/package/src/testHelpers/index.js | 1 + .../src/testHelpers/matchers/getElement.js | 3 + .../package/src/testHelpers/matchers/index.js | 11 ++ .../matchers/toContainContentElementBox.js | 61 ++++++++++ .../matchers/toContainFitViewport.js | 38 ++++++ 8 files changed, 166 insertions(+), 155 deletions(-) create mode 100644 entry_types/scrolled/package/src/testHelpers/matchers/getElement.js create mode 100644 entry_types/scrolled/package/src/testHelpers/matchers/index.js create mode 100644 entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js create mode 100644 entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js diff --git a/entry_types/scrolled/package/spec/contentElements/inlineImage/InlineImage-spec.js b/entry_types/scrolled/package/spec/contentElements/inlineImage/InlineImage-spec.js index a817757692..c2075fcb1e 100644 --- a/entry_types/scrolled/package/spec/contentElements/inlineImage/InlineImage-spec.js +++ b/entry_types/scrolled/package/spec/contentElements/inlineImage/InlineImage-spec.js @@ -1,7 +1,6 @@ import React from 'react'; import 'contentElements/inlineImage/frontend'; -import {renderContentElement, usePageObjects} from 'support/pageObjects'; -import {renderInContentElement} from 'pageflow-scrolled/testHelpers'; +import {renderInContentElement, useContentElementMatchers} from 'pageflow-scrolled/testHelpers'; import '@testing-library/jest-dom/extend-expect' import {InlineImage} from 'contentElements/inlineImage/InlineImage'; @@ -11,16 +10,26 @@ import {usePortraitOrientation} from 'frontend/usePortraitOrientation'; jest.mock('frontend/usePortraitOrientation'); describe('InlineImage', () => { - usePageObjects(); + useContentElementMatchers(); beforeEach(() => { usePortraitOrientation.mockReturnValue(false); }); + function renderInlineImage({contentElementWidth = 0, configuration = {id: 100}, ...seedOptions} = {}) { + const result = renderInContentElement( + , + {seed: seedOptions} + ); + result.simulateScrollPosition('near viewport'); + return result; + } + describe('circle crop', () => { it('forces 1:1 aspect ratio', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, imageModifiers: [ @@ -34,13 +43,11 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('square'); + expect(container).toContainFitViewport({aspectRatio: 'square'}); }); it('applies circle border radius', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, imageModifiers: [ @@ -54,13 +61,11 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getBoxBorderRadius()).toEqual('circle'); + expect(container).toContainContentElementBox({borderRadius: 'circle'}); }); it('applies box shadow on circle box', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, boxShadow: 'md', @@ -75,14 +80,11 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getBoxBorderRadius()).toEqual('circle'); - expect(contentElement.hasBoxShadow('md')).toBe(true); + expect(container).toContainContentElementBox({borderRadius: 'circle', boxShadow: 'md'}); }); it('overrides rounded styles', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, imageModifiers: [ @@ -97,15 +99,13 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getBoxBorderRadius()).toEqual('circle'); + expect(container).toContainContentElementBox({borderRadius: 'circle'}); }); }); describe('regular crop', () => { it('applies aspect ratio from crop value', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, imageModifiers: [ @@ -119,13 +119,11 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('square'); + expect(container).toContainFitViewport({aspectRatio: 'square'}); }); it('applies box shadow on outer box with rounded styles', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, boxShadow: 'lg', @@ -140,14 +138,11 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getBoxBorderRadius()).toEqual('md'); - expect(contentElement.hasBoxShadow('lg')).toBe(true); + expect(container).toContainContentElementBox({borderRadius: 'md', boxShadow: 'lg'}); }); it('applies rounded styles independently', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, imageModifiers: [ @@ -162,9 +157,8 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('square'); - expect(contentElement.getBoxBorderRadius()).toEqual('md'); + expect(container).toContainFitViewport({aspectRatio: 'square'}); + expect(container).toContainContentElementBox({borderRadius: 'md'}); }); }); @@ -172,8 +166,7 @@ describe('InlineImage', () => { it('applies circle crop when in portrait orientation', () => { usePortraitOrientation.mockReturnValue(true); - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, portraitId: 101, @@ -198,16 +191,14 @@ describe('InlineImage', () => { ] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('square'); - expect(contentElement.getBoxBorderRadius()).toEqual('circle'); + expect(container).toContainFitViewport({aspectRatio: 'square'}); + expect(container).toContainContentElementBox({borderRadius: 'circle'}); }); it('applies landscape modifiers when not in portrait orientation', () => { usePortraitOrientation.mockReturnValue(false); - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100, portraitId: 101, @@ -233,26 +224,14 @@ describe('InlineImage', () => { ] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('wide'); - expect(contentElement.getBoxBorderRadius()).toEqual('md'); + expect(container).toContainFitViewport({aspectRatio: 'wide'}); + expect(container).toContainContentElementBox({borderRadius: 'md'}); }); }); describe('srcset', () => { useFakeFeatures('frontend', ['image_srcset']); - function renderInlineImage({contentElementWidth = 0, ...seedOptions} = {}) { - const result = renderInContentElement( - , - {seed: seedOptions} - ); - result.simulateScrollPosition('near viewport'); - return result; - } - it('uses medium and large srcset for default width', () => { const {getByRole} = renderInlineImage({ imageFileUrlTemplates: { @@ -352,8 +331,7 @@ describe('InlineImage', () => { describe('basic functionality', () => { it('renders with FitViewport and ContentElementBox', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100 }, @@ -364,27 +342,23 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.hasFitViewport()).toBe(true); - expect(contentElement.containsBox()).toBe(true); + expect(container).toContainFitViewport(); + expect(container).toContainContentElementBox(); }); it('uses default aspect ratio when no crop modifier and no file', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { // No image file id }, imageFiles: [] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('0.75'); + expect(container).toContainFitViewport({aspectRatio: '0.75'}); }); it('uses default aspect ratio when file is not ready', () => { - const {getContentElement} = renderContentElement({ - typeName: 'inlineImage', + const {container} = renderInlineImage({ configuration: { id: 100 }, @@ -396,8 +370,7 @@ describe('InlineImage', () => { }] }); - const contentElement = getContentElement(); - expect(contentElement.getFitViewportAspectRatio()).toEqual('0.75'); + expect(container).toContainFitViewport({aspectRatio: '0.75'}); }); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/contentElementBox-spec.js b/entry_types/scrolled/package/spec/frontend/features/contentElementBox-spec.js index c124fc7dd4..022539f27d 100644 --- a/entry_types/scrolled/package/spec/frontend/features/contentElementBox-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/contentElementBox-spec.js @@ -3,10 +3,12 @@ import {frontend, ContentElementBox} from 'frontend'; import React from 'react'; import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useContentElementMatchers} from 'pageflow-scrolled/testHelpers'; import '@testing-library/jest-dom/extend-expect' describe('content element box', () => { usePageObjects(); + useContentElementMatchers(); beforeEach(() => { frontend.contentElementTypes.register('test', { @@ -91,7 +93,7 @@ describe('content element box', () => { } }); - expect(getContentElementByTestId('test').containsBox()).toEqual(true); + expect(getContentElementByTestId('test')).toContainContentElementBox(); }); it('does not render box for backdrop content element', () => { @@ -114,7 +116,7 @@ describe('content element box', () => { } }); - expect(getContentElementByTestId('test').containsBox()).toEqual(false); + expect(getContentElementByTestId('test')).not.toContainContentElementBox(); }); it('applies border radius class when borderRadius prop is provided', () => { @@ -126,8 +128,7 @@ describe('content element box', () => { } }); - const contentElement = getContentElementByTestId('testRounded'); - expect(contentElement.hasBoxBorderRadius('md')).toBe(true); + expect(getContentElementByTestId('testRounded')).toContainContentElementBox({borderRadius: 'md'}); }); it('does not render box when borderRadius is "none"', () => { @@ -139,7 +140,7 @@ describe('content element box', () => { } }); - expect(getContentElementByTestId('testNone').containsBox()).toEqual(false); + expect(getContentElementByTestId('testNone')).not.toContainContentElementBox(); }); it('applies box shadow CSS custom property from configuration', () => { @@ -152,7 +153,7 @@ describe('content element box', () => { } }); - expect(getContentElementByTestId('testBoxShadow').hasBoxShadow('md')).toBe(true); + expect(getContentElementByTestId('testBoxShadow')).toContainContentElementBox({boxShadow: 'md'}); }); it('applies outline color CSS custom property from configuration', () => { @@ -165,7 +166,7 @@ describe('content element box', () => { } }); - expect(getContentElementByTestId('testOutline').hasOutlineColor('#ff0000')).toBe(true); + expect(getContentElementByTestId('testOutline')).toContainContentElementBox({outlineColor: '#ff0000'}); }); it('renders box when borderRadius is "none" but configuration has boxShadow', () => { @@ -178,7 +179,7 @@ describe('content element box', () => { } }); - expect(getContentElementByTestId('testBoxShadowNoBorderRadius').containsBox()).toEqual(true); - expect(getContentElementByTestId('testBoxShadowNoBorderRadius').hasBoxShadow('md')).toBe(true); + expect(getContentElementByTestId('testBoxShadowNoBorderRadius')) + .toContainContentElementBox({boxShadow: 'md'}); }); }); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/index.js b/entry_types/scrolled/package/spec/support/pageObjects/index.js index 708e7c394b..5e207e56e8 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/index.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/index.js @@ -4,10 +4,8 @@ import {renderInEntry} from '..'; import {Entry} from 'frontend/Entry'; import foregroundStyles from 'frontend/Foreground.module.css'; import sharedTransitionStyles from 'frontend/transitions/shared.module.css'; -import contentElementBoxStyles from 'frontend/ContentElementBox.module.css'; import contentElementMarginStyles from 'frontend/ContentElementMargin.module.css'; import contentElementScrollSpaceStyles from 'frontend/ContentElementScrollSpace.module.css'; -import fitViewportStyles from 'frontend/FitViewport.module.css'; import centerLayoutStyles from 'frontend/layouts/Center.module.css'; import twoColumnLayoutStyles from 'frontend/layouts/TwoColumn.module.css'; import boxBoundaryMarginStyles from 'frontend/foregroundBoxes/BoxBoundaryMargin.module.css'; @@ -62,35 +60,6 @@ function mergeContentElement(seed, {ui, typeName, typeOptions = {}, permaId = 10 }; } -export function renderContentElement({typeName, configuration = {}, ...seedOptions} = {}) { - const seed = { - contentElements: [{ - permaId: 10, - typeName, - configuration - }], - ...seedOptions - }; - - const result = renderEntry({seed}); - - return { - ...result, - getContentElement() { - const el = result.container.querySelector('[class*="ContentElementMargin_module__wrapper"]'); - - if (!el) { - throw queryHelpers.getElementError( - `Unable to find content element with type ${typeName}.`, - result.container - ); - } - - return createContentElementPageObject(el); - } - }; -} - export function usePageObjects() { beforeEach(() => { jest.restoreAllMocks(); @@ -265,34 +234,7 @@ export function createContentElementPageObject(el) { const selectionRect = el.closest('[aria-label="Select content element"]'); return { - containsBox() { - return !!el.querySelector(`.${contentElementBoxStyles.wrapper}`); - }, - - hasBoxBorderRadius(value) { - const wrapper = el.querySelector(`.${contentElementBoxStyles.wrapper}`); - return wrapper && wrapper.style.getPropertyValue('--content-element-box-border-radius') === `var(--theme-content-element-box-border-radius-${value})`; - }, - - getBoxBorderRadius() { - const wrapper = el.querySelector(`.${contentElementBoxStyles.wrapper}`); - if (!wrapper) return null; - - const cssValue = wrapper.style.getPropertyValue('--content-element-box-border-radius'); - // Extract the value from var(--theme-content-element-box-border-radius-VALUE) - const match = cssValue.match(/var\(--theme-content-element-box-border-radius-(.+)\)/); - return match ? match[1] : cssValue; - }, - - hasBoxShadow(value) { - const wrapper = el.querySelector(`.${contentElementBoxStyles.wrapper}`); - return wrapper && wrapper.style.getPropertyValue('--content-element-box-shadow') === `var(--theme-content-element-box-shadow-${value})`; - }, - - hasOutlineColor(value) { - const wrapper = el.querySelector(`.${contentElementBoxStyles.wrapper}`); - return wrapper && wrapper.style.getPropertyValue('--content-element-box-outline-color') === value; - }, + el, hasMargin() { return !!el.closest(`.${contentElementMarginStyles.wrapper}`); @@ -322,10 +264,6 @@ export function createContentElementPageObject(el) { return !!el.closest(`.${contentElementScrollSpaceStyles.wrapper}`); }, - hasFitViewport() { - return !!el.querySelector(`.${fitViewportStyles.container}`); - }, - hasAlignment(alignment) { return !!( el.closest(`.${centerLayoutStyles[`align-${alignment}`]}`) || @@ -333,21 +271,6 @@ export function createContentElementPageObject(el) { ); }, - getFitViewportAspectRatio() { - const container = el.querySelector(`.${fitViewportStyles.container}`); - if (!container) return null; - - const cssValue = container.style.getPropertyValue('--fit-viewport-aspect-ratio'); - // Extract the value from var(--theme-aspect-ratio-VALUE) or return the raw value - const match = cssValue.match(/var\(--theme-aspect-ratio-(.+)\)/); - return match ? match[1] : cssValue; - }, - - isFitViewportOpaque() { - const container = el.querySelector(`.${fitViewportStyles.container}`); - return container?.classList.contains(fitViewportStyles.opaque); - }, - getMarginIndicator(position) { const {getByLabelText} = within(selectionRect); const labels = { diff --git a/entry_types/scrolled/package/src/testHelpers/index.js b/entry_types/scrolled/package/src/testHelpers/index.js index df58c70a95..21a7989eb2 100644 --- a/entry_types/scrolled/package/src/testHelpers/index.js +++ b/entry_types/scrolled/package/src/testHelpers/index.js @@ -1,3 +1,4 @@ +export * from './matchers'; export * from './normalizeSeed'; export * from './renderReactBasedBackboneView'; export * from './renderInContentElement'; diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/getElement.js b/entry_types/scrolled/package/src/testHelpers/matchers/getElement.js new file mode 100644 index 0000000000..e759bb01fa --- /dev/null +++ b/entry_types/scrolled/package/src/testHelpers/matchers/getElement.js @@ -0,0 +1,3 @@ +export function getElement(subject) { + return subject?.el ?? subject; +} diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/index.js b/entry_types/scrolled/package/src/testHelpers/matchers/index.js new file mode 100644 index 0000000000..473fe985f8 --- /dev/null +++ b/entry_types/scrolled/package/src/testHelpers/matchers/index.js @@ -0,0 +1,11 @@ +import {toContainContentElementBox} from './toContainContentElementBox'; +import {toContainFitViewport} from './toContainFitViewport'; + +export function useContentElementMatchers() { + beforeEach(() => { + expect.extend({ + toContainContentElementBox, + toContainFitViewport + }); + }); +} diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js b/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js new file mode 100644 index 0000000000..114b362039 --- /dev/null +++ b/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js @@ -0,0 +1,61 @@ +import styles from '../../frontend/ContentElementBox.module.css'; + +import {getElement} from './getElement'; + +export function toContainContentElementBox(subject, options = {}) { + const wrapper = getElement(subject).querySelector(`.${styles.wrapper}`); + + if (!wrapper) { + return { + pass: false, + message: () => 'expected element to contain a content element box, but found none' + }; + } + + const mismatches = boxPropertyMismatches(wrapper, options); + + return { + pass: mismatches.length === 0, + message: () => + mismatches.length + ? `expected content element box to have ${mismatches.join(', ')}` + : `expected element not to contain a content element box${describeOptions(options)}` + }; +} + +const boxProperties = { + boxShadow: { + customProperty: '--content-element-box-shadow', + expectedValue: value => `var(--theme-content-element-box-shadow-${value})` + }, + borderRadius: { + customProperty: '--content-element-box-border-radius', + expectedValue: value => `var(--theme-content-element-box-border-radius-${value})` + }, + outlineColor: { + customProperty: '--content-element-box-outline-color', + expectedValue: value => value + } +}; + +function boxPropertyMismatches(wrapper, options) { + return Object.keys(options).reduce((mismatches, name) => { + const property = boxProperties[name]; + + if (property) { + const expected = property.expectedValue(options[name]); + const actual = wrapper.style.getPropertyValue(property.customProperty); + + if (actual !== expected) { + mismatches.push(`${name} ${JSON.stringify(options[name])} (found ${JSON.stringify(actual)})`); + } + } + + return mismatches; + }, []); +} + +function describeOptions(options) { + const names = Object.keys(options).filter(name => boxProperties[name]); + return names.length ? ` with ${names.join(', ')}` : ''; +} diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js b/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js new file mode 100644 index 0000000000..9261c2e959 --- /dev/null +++ b/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js @@ -0,0 +1,38 @@ +import styles from '../../frontend/FitViewport.module.css'; + +import {getElement} from './getElement'; + +export function toContainFitViewport(subject, {aspectRatio} = {}) { + const container = getElement(subject).querySelector(`.${styles.container}`); + + if (!container) { + return { + pass: false, + message: () => 'expected element to contain a FitViewport, but found none' + }; + } + + if (aspectRatio !== undefined) { + const actual = getAspectRatio(container); + + if (actual !== aspectRatio) { + return { + pass: false, + message: () => + `expected FitViewport to have aspect ratio ${JSON.stringify(aspectRatio)} ` + + `(found ${JSON.stringify(actual)})` + }; + } + } + + return { + pass: true, + message: () => 'expected element not to contain a FitViewport' + }; +} + +function getAspectRatio(container) { + const cssValue = container.style.getPropertyValue('--fit-viewport-aspect-ratio'); + const match = cssValue.match(/var\(--theme-aspect-ratio-(.+)\)/); + return match ? match[1] : cssValue; +} From 430aa977e737967d0f99471e10393f09fe734310 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Tue, 9 Jun 2026 23:55:04 +0200 Subject: [PATCH 02/11] Add content element layout test matchers Extract the page object's margin, scroll space, and alignment predicates into internal jest matchers (toHaveContentElementMargin, toHaveScrollSpace, toHaveAlignment), registered via useContentElementLayoutMatchers. This keeps page objects focused on locating and acting, and lets the same assertions read consistently across specs. These stay internal to the package since they assert framework state applied around the content element rather than chrome the element itself controls. --- .../features/contentElementAlignment-spec.js | 11 ++-- .../features/contentElementMargin-spec.js | 38 ++++++------- .../contentElementScrollSpace-spec.js | 6 ++- .../package/spec/support/matchers/index.js | 13 +++++ .../spec/support/matchers/toHaveAlignment.js | 19 +++++++ .../matchers/toHaveContentElementMargin.js | 54 +++++++++++++++++++ .../support/matchers/toHaveScrollSpace.js | 14 +++++ .../package/spec/support/pageObjects/index.js | 37 ------------- 8 files changed, 129 insertions(+), 63 deletions(-) create mode 100644 entry_types/scrolled/package/spec/support/matchers/index.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveAlignment.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveContentElementMargin.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveScrollSpace.js diff --git a/entry_types/scrolled/package/spec/frontend/features/contentElementAlignment-spec.js b/entry_types/scrolled/package/spec/frontend/features/contentElementAlignment-spec.js index 000e031c55..d3f73ef140 100644 --- a/entry_types/scrolled/package/spec/frontend/features/contentElementAlignment-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/contentElementAlignment-spec.js @@ -1,8 +1,10 @@ import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useContentElementLayoutMatchers} from 'support/matchers'; import '@testing-library/jest-dom/extend-expect'; describe('content element alignment', () => { usePageObjects(); + useContentElementLayoutMatchers(); it('applies alignment class for narrow inline element', () => { const {getContentElementByTestId} = renderEntry({ @@ -18,8 +20,7 @@ describe('content element alignment', () => { } }); - const contentElement = getContentElementByTestId('probe'); - expect(contentElement.hasAlignment('right')).toBe(true); + expect(getContentElementByTestId('probe')).toHaveAlignment('right'); }); it('applies alignment class for narrow standAlone element', () => { @@ -41,8 +42,7 @@ describe('content element alignment', () => { } }); - const contentElement = getContentElementByTestId('probe'); - expect(contentElement.hasAlignment('right')).toBe(true); + expect(getContentElementByTestId('probe')).toHaveAlignment('right'); }); it('ignores alignment class when width changes', () => { @@ -59,7 +59,6 @@ describe('content element alignment', () => { } }); - const contentElement = getContentElementByTestId('probe'); - expect(contentElement.hasAlignment('left')).toBe(false); + expect(getContentElementByTestId('probe')).not.toHaveAlignment('left'); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/contentElementMargin-spec.js b/entry_types/scrolled/package/spec/frontend/features/contentElementMargin-spec.js index 1df2a6d6bb..10245bd8aa 100644 --- a/entry_types/scrolled/package/spec/frontend/features/contentElementMargin-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/contentElementMargin-spec.js @@ -1,11 +1,13 @@ import React from 'react'; import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useContentElementLayoutMatchers} from 'support/matchers'; import {contentElementWidths as widths, frontend} from 'pageflow-scrolled/frontend'; describe('content element margin', () => { usePageObjects(); + useContentElementLayoutMatchers(); it('applies margin to content elements by default', () => { const {getContentElementByTestId} = renderEntry({ @@ -17,7 +19,7 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).hasMargin()).toBe(true); + expect(getContentElementByTestId(1)).toHaveContentElementMargin(); }); it('does not apply margin to full width content elements', () => { @@ -30,7 +32,7 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).hasMargin()).toBe(false); + expect(getContentElementByTestId(1)).not.toHaveContentElementMargin(); }); it('does not apply top margin to first content element in section', () => { @@ -44,8 +46,8 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).hasTopMargin()).toBe(false); - expect(getContentElementByTestId(2).hasTopMargin()).toBe(true); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({topTrimmed: true}); + expect(getContentElementByTestId(2)).toHaveContentElementMargin({topTrimmed: false}); }); it('does not trim custom margin top on first content element in section', () => { @@ -58,7 +60,7 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).hasTopMargin()).toBe(true); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({topTrimmed: false}); }); it('still applies top margin to first content element in cards appearance', () => { @@ -72,8 +74,8 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).hasTopMargin()).toBe(true); - expect(getContentElementByTestId(2).hasTopMargin()).toBe(true); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({topTrimmed: false}); + expect(getContentElementByTestId(2)).toHaveContentElementMargin({topTrimmed: false}); }); it('supports defaultMarginTop option in content element registration', () => { @@ -93,7 +95,7 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).getMarginTop()).toBe('1.375rem'); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({top: '1.375rem'}); }); it('sets margin top via --margin-top custom property', () => { @@ -106,7 +108,7 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).getMarginTop()).toBe('var(--theme-content-element-margin-xl)'); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({top: 'var(--theme-content-element-margin-xl)'}); }); it('sets margin bottom via --prev-margin-bottom on next element', () => { @@ -120,8 +122,8 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).getMarginBottom()).toBe(''); - expect(getContentElementByTestId(2).getPrevMarginBottom()).toBe('var(--theme-content-element-margin-lg)'); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({bottom: ''}); + expect(getContentElementByTestId(2)).toHaveContentElementMargin({prevBottom: 'var(--theme-content-element-margin-lg)'}); }); it('applies --margin-bottom when next element has different width', () => { @@ -136,8 +138,8 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(2).getMarginBottom()).toBe('var(--theme-content-element-margin-lg)'); - expect(getContentElementByTestId(3).getPrevMarginBottom()).toBe(''); + expect(getContentElementByTestId(2)).toHaveContentElementMargin({bottom: 'var(--theme-content-element-margin-lg)'}); + expect(getContentElementByTestId(3)).toHaveContentElementMargin({prevBottom: ''}); }); it('passes previous inline element margin bottom skipping side elements', () => { @@ -152,9 +154,9 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).getMarginBottom()).toBe(''); - expect(getContentElementByTestId(2).getPrevMarginBottom()).toBe(''); - expect(getContentElementByTestId(3).getPrevMarginBottom()).toBe('var(--theme-content-element-margin-lg)'); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({bottom: ''}); + expect(getContentElementByTestId(2)).toHaveContentElementMargin({prevBottom: ''}); + expect(getContentElementByTestId(3)).toHaveContentElementMargin({prevBottom: 'var(--theme-content-element-margin-lg)'}); }); it('sets margin top via --margin-top custom property in center layout', () => { @@ -167,7 +169,7 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).getMarginTop()).toBe('var(--theme-content-element-margin-xl)'); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({top: 'var(--theme-content-element-margin-xl)'}); }); it('sets margin bottom via --margin-bottom custom property in center layout', () => { @@ -180,6 +182,6 @@ describe('content element margin', () => { } }); - expect(getContentElementByTestId(1).getMarginBottom()).toBe('var(--theme-content-element-margin-xl)'); + expect(getContentElementByTestId(1)).toHaveContentElementMargin({bottom: 'var(--theme-content-element-margin-xl)'}); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/contentElementScrollSpace-spec.js b/entry_types/scrolled/package/spec/frontend/features/contentElementScrollSpace-spec.js index 540ffe2215..935c1a7a77 100644 --- a/entry_types/scrolled/package/spec/frontend/features/contentElementScrollSpace-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/contentElementScrollSpace-spec.js @@ -1,7 +1,9 @@ import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useContentElementLayoutMatchers} from 'support/matchers'; describe('content element scroll space', () => { usePageObjects(); + useContentElementLayoutMatchers(); it('does not apply scroll space to elements by default', () => { const {getContentElementByTestId} = renderEntry({ @@ -15,7 +17,7 @@ describe('content element scroll space', () => { } }); - expect(getContentElementByTestId(1).hasScrollSpace()).toBe(false); + expect(getContentElementByTestId(1)).not.toHaveScrollSpace(); }); it('applies scroll space to elements with stand alone position', () => { @@ -31,6 +33,6 @@ describe('content element scroll space', () => { } }); - expect(getContentElementByTestId(1).hasScrollSpace()).toBe(true); + expect(getContentElementByTestId(1)).toHaveScrollSpace(); }); }); diff --git a/entry_types/scrolled/package/spec/support/matchers/index.js b/entry_types/scrolled/package/spec/support/matchers/index.js new file mode 100644 index 0000000000..81a988edae --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/index.js @@ -0,0 +1,13 @@ +import {toHaveContentElementMargin} from './toHaveContentElementMargin'; +import {toHaveScrollSpace} from './toHaveScrollSpace'; +import {toHaveAlignment} from './toHaveAlignment'; + +export function useContentElementLayoutMatchers() { + beforeEach(() => { + expect.extend({ + toHaveContentElementMargin, + toHaveScrollSpace, + toHaveAlignment + }); + }); +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveAlignment.js b/entry_types/scrolled/package/spec/support/matchers/toHaveAlignment.js new file mode 100644 index 0000000000..2b6bc72126 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveAlignment.js @@ -0,0 +1,19 @@ +import centerLayoutStyles from 'frontend/layouts/Center.module.css'; +import twoColumnLayoutStyles from 'frontend/layouts/TwoColumn.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveAlignment(subject, alignment) { + const el = getElement(subject); + const pass = !!( + el.closest(`.${centerLayoutStyles[`align-${alignment}`]}`) || + el.closest(`.${twoColumnLayoutStyles[`align-${alignment}`]}`) + ); + + return { + pass, + message: () => pass + ? `expected element not to have alignment ${JSON.stringify(alignment)}` + : `expected element to have alignment ${JSON.stringify(alignment)}` + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveContentElementMargin.js b/entry_types/scrolled/package/spec/support/matchers/toHaveContentElementMargin.js new file mode 100644 index 0000000000..4c2f33bc7e --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveContentElementMargin.js @@ -0,0 +1,54 @@ +import styles from 'frontend/ContentElementMargin.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveContentElementMargin(subject, options = {}) { + const wrapper = getElement(subject).closest(`.${styles.wrapper}`); + + if (!wrapper) { + return { + pass: false, + message: () => 'expected element to have a content element margin, but found none' + }; + } + + const mismatches = marginMismatches(wrapper, options); + + return { + pass: mismatches.length === 0, + message: () => + mismatches.length + ? `expected content element margin to have ${mismatches.join(', ')}` + : 'expected element not to have a content element margin' + }; +} + +const customProperties = { + top: '--margin-top', + bottom: '--margin-bottom', + prevBottom: '--prev-margin-bottom' +}; + +function marginMismatches(wrapper, options) { + const mismatches = []; + + Object.keys(customProperties).forEach(name => { + if (name in options) { + const actual = wrapper.style.getPropertyValue(customProperties[name]); + + if (actual !== options[name]) { + mismatches.push(`${name} ${JSON.stringify(options[name])} (found ${JSON.stringify(actual)})`); + } + } + }); + + if ('topTrimmed' in options) { + const actual = wrapper.classList.contains(styles.noTopMargin); + + if (actual !== options.topTrimmed) { + mismatches.push(`topTrimmed ${JSON.stringify(options.topTrimmed)} (found ${JSON.stringify(actual)})`); + } + } + + return mismatches; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveScrollSpace.js b/entry_types/scrolled/package/spec/support/matchers/toHaveScrollSpace.js new file mode 100644 index 0000000000..806fca9146 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveScrollSpace.js @@ -0,0 +1,14 @@ +import styles from 'frontend/ContentElementScrollSpace.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveScrollSpace(subject) { + const pass = !!getElement(subject).closest(`.${styles.wrapper}`); + + return { + pass, + message: () => pass + ? 'expected element not to have scroll space' + : 'expected element to have scroll space' + }; +} diff --git a/entry_types/scrolled/package/spec/support/pageObjects/index.js b/entry_types/scrolled/package/spec/support/pageObjects/index.js index 5e207e56e8..95951c9688 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/index.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/index.js @@ -4,8 +4,6 @@ import {renderInEntry} from '..'; import {Entry} from 'frontend/Entry'; import foregroundStyles from 'frontend/Foreground.module.css'; import sharedTransitionStyles from 'frontend/transitions/shared.module.css'; -import contentElementMarginStyles from 'frontend/ContentElementMargin.module.css'; -import contentElementScrollSpaceStyles from 'frontend/ContentElementScrollSpace.module.css'; import centerLayoutStyles from 'frontend/layouts/Center.module.css'; import twoColumnLayoutStyles from 'frontend/layouts/TwoColumn.module.css'; import boxBoundaryMarginStyles from 'frontend/foregroundBoxes/BoxBoundaryMargin.module.css'; @@ -236,41 +234,6 @@ export function createContentElementPageObject(el) { return { el, - hasMargin() { - return !!el.closest(`.${contentElementMarginStyles.wrapper}`); - }, - - hasTopMargin() { - const wrapper = el.closest(`.${contentElementMarginStyles.wrapper}`); - return wrapper && !wrapper.classList.contains(contentElementMarginStyles.noTopMargin); - }, - - getMarginTop() { - const wrapper = el.closest(`.${contentElementMarginStyles.wrapper}`); - return wrapper && wrapper.style.getPropertyValue('--margin-top'); - }, - - getMarginBottom() { - const wrapper = el.closest(`.${contentElementMarginStyles.wrapper}`); - return wrapper && wrapper.style.getPropertyValue('--margin-bottom'); - }, - - getPrevMarginBottom() { - const wrapper = el.closest(`.${contentElementMarginStyles.wrapper}`); - return wrapper && wrapper.style.getPropertyValue('--prev-margin-bottom'); - }, - - hasScrollSpace() { - return !!el.closest(`.${contentElementScrollSpaceStyles.wrapper}`); - }, - - hasAlignment(alignment) { - return !!( - el.closest(`.${centerLayoutStyles[`align-${alignment}`]}`) || - el.closest(`.${twoColumnLayoutStyles[`align-${alignment}`]}`) - ); - }, - getMarginIndicator(position) { const {getByLabelText} = within(selectionRect); const labels = { From 753e28e6d9cc283b75dbe4ee826cf943f3327172 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 08:59:30 +0200 Subject: [PATCH 03/11] Add testing conventions guide and matcher docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give contributors a single written reference for choosing a render helper, custom wrappers, setup hooks, the public/internal matcher split, and spec layout — complementing the generated JSDoc, which is less accessible outside the docs site. Surface the public content element matchers and the useContentElementMatchers hook in the generated API reference under a Test Helpers heading so content element plugin authors can discover them. --- .../doc/internal/testing_conventions.md | 35 +++++++++ .../internal/testing_conventions/matchers.md | 63 ++++++++++++++++ .../testing_conventions/render-helpers.md | 74 +++++++++++++++++++ .../testing_conventions/spec-layout.md | 73 ++++++++++++++++++ .../scrolled/package/documentation.yml | 8 +- .../package/src/testHelpers/matchers/index.js | 20 +++++ .../matchers/toContainContentElementBox.js | 19 +++++ .../matchers/toContainFitViewport.js | 18 +++++ 8 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 entry_types/scrolled/doc/internal/testing_conventions.md create mode 100644 entry_types/scrolled/doc/internal/testing_conventions/matchers.md create mode 100644 entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md create mode 100644 entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md diff --git a/entry_types/scrolled/doc/internal/testing_conventions.md b/entry_types/scrolled/doc/internal/testing_conventions.md new file mode 100644 index 0000000000..60f5978d26 --- /dev/null +++ b/entry_types/scrolled/doc/internal/testing_conventions.md @@ -0,0 +1,35 @@ +# 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, custom wrappers, 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. diff --git a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md new file mode 100644 index 0000000000..2c2b297d0f --- /dev/null +++ b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md @@ -0,0 +1,63 @@ +# 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**. + +**Public matchers** (`src/testHelpers/matchers/`, shipped via +`pageflow-scrolled/testHelpers`, enabled with `useContentElementMatchers()`) +cover chrome a content element opts into through framework components. +Plugin authors need these to test their own components. + +| Matcher | Asserts | +| --- | --- | +| `toContainContentElementBox({boxShadow, borderRadius, outlineColor})` | A `` is present (bare call), and the requested theme styles are applied. `.not` asserts absence. | +| `toContainFitViewport({aspectRatio})` | A `` is present (bare call) with the requested aspect ratio. `.not` asserts absence. | + +**Internal matchers** (`spec/support/matchers/`, enabled with +`useContentElementLayoutMatchers()`) cover framework state applied +*around* the element. They depend on the entry-level layout, so the +subject must come from `renderEntry` — `renderInContentElement` does not +render these wrappers. + +| Matcher | Asserts | +| --- | --- | +| `toHaveContentElementMargin({top, bottom, prevBottom, topTrimmed})` | A content element margin wrapper is present; the `--margin-*` custom properties (raw CSS strings) match; `topTrimmed` checks the trim class. `.not` asserts absence. | +| `toHaveScrollSpace()` | The element sits inside a scroll-space wrapper. | +| `toHaveAlignment(value)` | The element carries the alignment class for the given value. | + +Both sets are registered through the `useXxx` hooks 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 +public matchers through `useContentElementMatchers` and internal ones +through `useContentElementLayoutMatchers` so specs opt in explicitly, +mirroring `usePageObjects`. diff --git a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md new file mode 100644 index 0000000000..fe43c1866d --- /dev/null +++ b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md @@ -0,0 +1,74 @@ +# 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. They +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. + +| Helper | Import from | Scope | +| --- | --- | --- | +| `renderInEntry` | `pageflow-scrolled/testHelpers` | Entry state + `RootProviders`. For any component that needs entry state but nothing more. | +| `renderHookInEntry` | `pageflow-scrolled/testHelpers` | Same as `renderInEntry`, for selector hooks that read entry state. | +| `renderInContentElement` | `pageflow-scrolled/testHelpers` | Content-element scope: attributes, lifecycle, command emitter, optional inline-editing context. For components rendered *inside* a content element. | +| `renderWithReviewState` | `pageflow-scrolled/testHelpers` | `ReviewStateProvider` only. For code in `src/review/`. | +| `renderEntry` | `support/pageObjects` | Full `Entry`. Returns page-object queries (`getContentElementByTestId`, `getSectionByPermaId`). For cross-cutting frontend features and integration behavior. | + +## Picking a helper + +Pick the smallest-scope helper sufficient for the behavior under test. +The path of the spec file is the strongest signal: `spec/-spec.js` +mirrors `src/.js`, and the scope at that path determines which +helper to use. + +- A content element's component (`src/contentElements//.js`) + → `renderInContentElement`, asserting with [matchers](matchers.md). +- A reusable frontend component (`src/frontend/.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`, and `simulateStorylineMode`. Its `inlineEditing` +option opts the element into an inline-editing context — pass `true` for +editable defaults or an object (`{isEditable, isSelected, +transientState}`) to override. + +## Custom wrappers + +`renderInEntry` and `renderInContentElement` accept a `wrapper` option: a +`({children}) => JSX` component that adds extra providers around the +rendered tree. Use it for a one-off provider combination a test needs. + +When a combination is common enough to be discoverable, add a named +option to the render helper that installs the providers internally — +this is how `renderInContentElement`'s `inlineEditing` option works — +rather than repeating the same custom `wrapper` across specs. + +## 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 layout [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.) diff --git a/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md b/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md new file mode 100644 index 0000000000..8f4e4a39a3 --- /dev/null +++ b/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md @@ -0,0 +1,73 @@ +# 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. Two rules govern when a spec becomes a +directory: + +1. A **`src/Foo.js`** source file corresponds to a **`spec/Foo-spec.js`** + spec file. When it grows large enough to split into topics, it becomes + a directory: topic files live at `spec/Foo/features/-spec.js`. +2. A **`src/Foo/`** source directory corresponds to a **`spec/Foo/`** spec + directory, with one `spec/Foo/-spec.js` per + `src/Foo/.js`. Topic splits of the main component live in + `spec/Foo/features/` alongside the helper specs. + +The `features/` sub-namespace is the *only* place topic splits live, +regardless of whether the unit under test is a `.js` file or a `/` +directory. This keeps spec layout invariant under +`src/Foo.js → src/Foo/index.js` refactorings. + +A *unit spec* mirrors a single source unit and stays at the directory +level, named after that unit. A unit is usually a separate file +(`spec/Foo/-spec.js` for `src/Foo/.js`), but can also be +a **named export** of the main file — e.g. `AudioPlayer/index.js` +exports both the `AudioPlayer` component and a `processSources` helper, +so `processSources-spec.js` mirrors that export and is *not* a topic +split. (`structuredData-spec.js` in the same directory, which exercises +the component's behavior, is.) + +`features/` holds the remaining *behavior-grouped* tests, written +against the unit's stable public interface; these tend to outlive the +internal helpers they exercise. The path's scope and context determine +which render helper and setup hooks the specs in that directory use. + +**`features/` attaches to the level of the unit under test.** The +directory path identifies the unit — whatever has a source counterpart +at that path — and `features/` nests *under* it to group that unit's +behavior topics; it never floats up to the context root. So the +`EditableText` component (`src/frontend/inlineEditing/EditableText/`) +owns `spec/frontend/inlineEditing/EditableText/features/`, with its +helper specs (`blocks-spec.js`, `marks-spec.js`, …) alongside. +Conversely, behavior with no single source counterpart in a context — +the inline-editing integration itself (`contentElementSelection`, +`marginIndicator`) — lives directly in that context's +`spec/frontend/inlineEditing/features/`. The render 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. + +## Placement across contexts + +The directory path encodes the *context* a spec runs in: specs under +`spec/frontend/inlineEditing/` load the inline-editing extension, those +under `spec/frontend/commenting/` the commenting extension, and those +directly under `spec/frontend/` neither. When a unit behaves differently +across these contexts, it gets one spec per context, named after the +unit, in the matching directory — the path conveys the context, so the +filenames stay identical. + +Prefer committing to the public interface over its wiring. When a unit's +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`, rather than white-box-driving +the replacement mechanism (which couples the spec to how providers are +registered, not to the contract). For example `usePhonePlatform` is +asserted via a probe content element in +`spec/frontend/features/usePhonePlatform-spec.js` (default provider, +reads the `browser`) and +`spec/frontend/inlineEditing/features/usePhonePlatform-spec.js` (the +inline-editing provider, driven by the editor's emulation-mode +messages). Both live in `features/` because they exercise rendered +behavior; a spec that instead drove the hook directly with +`renderHookInEntry` would be a dir-level white-box unit spec. diff --git a/entry_types/scrolled/package/documentation.yml b/entry_types/scrolled/package/documentation.yml index 327b66b117..03f157526a 100644 --- a/entry_types/scrolled/package/documentation.yml +++ b/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. diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/index.js b/entry_types/scrolled/package/src/testHelpers/matchers/index.js index 473fe985f8..32bb1e54c2 100644 --- a/entry_types/scrolled/package/src/testHelpers/matchers/index.js +++ b/entry_types/scrolled/package/src/testHelpers/matchers/index.js @@ -1,6 +1,26 @@ import {toContainContentElementBox} from './toContainContentElementBox'; import {toContainFitViewport} from './toContainFitViewport'; +/** + * Register the public content element matchers for the surrounding + * `describe` block: {@link toContainContentElementBox} and + * {@link toContainFitViewport}. + * + * Call inside a `describe` block. Content element plugins can use this to + * assert that their component opts into the framework chrome correctly. + * + * @example + * import {renderInContentElement, useContentElementMatchers} from 'pageflow-scrolled/testHelpers'; + * + * describe('MyContentElement', () => { + * useContentElementMatchers(); + * + * it('renders inside a box', () => { + * const {container} = renderInContentElement(); + * expect(container).toContainContentElementBox(); + * }); + * }); + */ export function useContentElementMatchers() { beforeEach(() => { expect.extend({ diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js b/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js index 114b362039..0ebc717f49 100644 --- a/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js +++ b/entry_types/scrolled/package/src/testHelpers/matchers/toContainContentElementBox.js @@ -2,6 +2,25 @@ import styles from '../../frontend/ContentElementBox.module.css'; import {getElement} from './getElement'; +/** + * Assert that the subject contains a `ContentElementBox` and, optionally, + * that the box has specific theme styles applied. Registered via + * {@link useContentElementMatchers}. + * + * The subject is a DOM element, e.g. the `container` returned by + * {@link renderInContentElement}. Negate with `.not` to assert that no + * box is present. + * + * @param {Object} [options] + * @param {string} [options.boxShadow] - Expected box shadow theme scale, e.g. `'md'`. + * @param {string} [options.borderRadius] - Expected border radius theme scale, e.g. `'circle'`. + * @param {string} [options.outlineColor] - Expected outline color. + * + * @example + * expect(container).toContainContentElementBox(); + * expect(container).toContainContentElementBox({borderRadius: 'circle', boxShadow: 'md'}); + * expect(container).not.toContainContentElementBox(); + */ export function toContainContentElementBox(subject, options = {}) { const wrapper = getElement(subject).querySelector(`.${styles.wrapper}`); diff --git a/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js b/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js index 9261c2e959..8f65645b6b 100644 --- a/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js +++ b/entry_types/scrolled/package/src/testHelpers/matchers/toContainFitViewport.js @@ -2,6 +2,24 @@ import styles from '../../frontend/FitViewport.module.css'; import {getElement} from './getElement'; +/** + * Assert that the subject contains a `FitViewport` and, optionally, that + * it has a specific aspect ratio. Registered via + * {@link useContentElementMatchers}. + * + * The subject is a DOM element, e.g. the `container` returned by + * {@link renderInContentElement}. Negate with `.not` to assert that no + * `FitViewport` is present. + * + * @param {Object} [options] + * @param {string} [options.aspectRatio] - + * Expected aspect ratio: a theme scale name (e.g. `'square'`) or a raw + * ratio value (e.g. `'0.75'`). + * + * @example + * expect(container).toContainFitViewport(); + * expect(container).toContainFitViewport({aspectRatio: 'square'}); + */ export function toContainFitViewport(subject, {aspectRatio} = {}) { const container = getElement(subject).querySelector(`.${styles.container}`); From 303e604b3bade552c04fe6ca8ae2f0b87fc2192f Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 09:22:24 +0200 Subject: [PATCH 04/11] Group topic-split specs under features directories Move specs that split a single source file's tests by topic into a features/ subdirectory, matching the spec-layout convention. This keeps behavior-grouped specs visually distinct from unit specs that mirror a source file or a named export. --- .../frontend/ExternalLinkList/{ => features}/backfaces-spec.js | 0 .../frontend/ExternalLinkList/{ => features}/boxStyles-spec.js | 0 .../ExternalLinkList/{ => features}/editorSelection-spec.js | 0 .../frontend/ExternalLinkList/{ => features}/placeholder-spec.js | 0 .../hotspots/Hotspots/{ => features}/activeImage-spec.js | 0 .../hotspots/Hotspots/{ => features}/areaClipPaths-spec.js | 0 .../hotspots/Hotspots/{ => features}/areaOutlines-spec.js | 0 .../hotspots/Hotspots/{ => features}/customProperties-spec.js | 0 .../hotspots/Hotspots/{ => features}/editorCommands-spec.js | 0 .../hotspots/Hotspots/{ => features}/editorTransientState-spec.js | 0 .../hotspots/Hotspots/{ => features}/imageRendering-spec.js | 0 .../hotspots/Hotspots/{ => features}/indicators-spec.js | 0 .../hotspots/Hotspots/{ => features}/panZoomScroller-spec.js | 0 .../hotspots/Hotspots/{ => features}/resizeObserving-spec.js | 0 .../hotspots/Hotspots/{ => features}/tooltipDisplay-spec.js | 0 .../hotspots/Hotspots/{ => features}/tooltipReferences-spec.js | 0 .../hotspots/Hotspots/{ => features}/tooltipRendering-spec.js | 0 .../DefaultNavigation/{ => features}/chapters-spec.js | 0 .../DefaultNavigation/{ => features}/components-spec.js | 0 .../DefaultNavigation/{ => features}/logo-spec.js | 0 .../DefaultNavigation/{ => features}/menu-spec.js | 0 .../DefaultNavigation/{ => features}/mobileMenu-spec.js | 0 .../DefaultNavigation/{ => features}/styling-spec.js | 0 23 files changed, 0 insertions(+), 0 deletions(-) rename entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/{ => features}/backfaces-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/{ => features}/boxStyles-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/{ => features}/editorSelection-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/{ => features}/placeholder-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/activeImage-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/areaClipPaths-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/areaOutlines-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/customProperties-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/editorCommands-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/editorTransientState-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/imageRendering-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/indicators-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/panZoomScroller-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/resizeObserving-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/tooltipDisplay-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/tooltipReferences-spec.js (100%) rename entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/{ => features}/tooltipRendering-spec.js (100%) rename entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/{ => features}/chapters-spec.js (100%) rename entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/{ => features}/components-spec.js (100%) rename entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/{ => features}/logo-spec.js (100%) rename entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/{ => features}/menu-spec.js (100%) rename entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/{ => features}/mobileMenu-spec.js (100%) rename entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/{ => features}/styling-spec.js (100%) diff --git a/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/backfaces-spec.js b/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/backfaces-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/backfaces-spec.js rename to entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/backfaces-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/boxStyles-spec.js b/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/boxStyles-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/boxStyles-spec.js rename to entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/boxStyles-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/editorSelection-spec.js b/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/editorSelection-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/editorSelection-spec.js rename to entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/editorSelection-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/placeholder-spec.js b/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/placeholder-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/placeholder-spec.js rename to entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/features/placeholder-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/activeImage-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/activeImage-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/activeImage-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/activeImage-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/areaClipPaths-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/areaClipPaths-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/areaClipPaths-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/areaClipPaths-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/areaOutlines-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/areaOutlines-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/areaOutlines-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/areaOutlines-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/customProperties-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/customProperties-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/customProperties-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/customProperties-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/editorCommands-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/editorCommands-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/editorCommands-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/editorCommands-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/editorTransientState-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/editorTransientState-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/editorTransientState-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/editorTransientState-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/imageRendering-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/imageRendering-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/imageRendering-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/imageRendering-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/indicators-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/indicators-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/indicators-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/indicators-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/panZoomScroller-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/panZoomScroller-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/panZoomScroller-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/panZoomScroller-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/resizeObserving-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/resizeObserving-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/resizeObserving-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/resizeObserving-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/tooltipDisplay-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/tooltipDisplay-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/tooltipDisplay-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/tooltipDisplay-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/tooltipReferences-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/tooltipReferences-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/tooltipReferences-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/tooltipReferences-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/tooltipRendering-spec.js b/entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/tooltipRendering-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/tooltipRendering-spec.js rename to entry_types/scrolled/package/spec/contentElements/hotspots/Hotspots/features/tooltipRendering-spec.js diff --git a/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/chapters-spec.js b/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/chapters-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/chapters-spec.js rename to entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/chapters-spec.js diff --git a/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/components-spec.js b/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/components-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/components-spec.js rename to entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/components-spec.js diff --git a/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/logo-spec.js b/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/logo-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/logo-spec.js rename to entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/logo-spec.js diff --git a/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/menu-spec.js b/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/menu-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/menu-spec.js rename to entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/menu-spec.js diff --git a/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/mobileMenu-spec.js b/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/mobileMenu-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/mobileMenu-spec.js rename to entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/mobileMenu-spec.js diff --git a/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/styling-spec.js b/entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/styling-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/styling-spec.js rename to entry_types/scrolled/package/spec/widgets/defaultNavigation/DefaultNavigation/features/styling-spec.js From 4cd880282bac6e77f0328b97efb67a8a28fab477 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 09:22:24 +0200 Subject: [PATCH 05/11] Test usePhonePlatform via a fake content element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the white-box hook specs with feature specs that render a probe content element through renderEntry and assert the hook's observable value — in the frontend context (default provider, reads the browser) and the inline-editing context (provider driven by the editor's emulation-mode messages). This commits the tests to the usePhonePlatform interface rather than the provider-replacement wiring, so the providers can be restructured without breaking the specs. --- .../features/usePhonePlatform-spec.js | 30 +++++++++++++ .../features/usePhonePlatform-spec.js | 42 +++++++++++++++++++ .../usePhonePlatform/inEditorPreview-spec.js | 38 ----------------- .../usePhonePlatform/inFrontend-spec.js | 22 ---------- 4 files changed, 72 insertions(+), 60 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/features/usePhonePlatform-spec.js create mode 100644 entry_types/scrolled/package/spec/frontend/inlineEditing/features/usePhonePlatform-spec.js delete mode 100644 entry_types/scrolled/package/spec/frontend/usePhonePlatform/inEditorPreview-spec.js delete mode 100644 entry_types/scrolled/package/spec/frontend/usePhonePlatform/inFrontend-spec.js diff --git a/entry_types/scrolled/package/spec/frontend/features/usePhonePlatform-spec.js b/entry_types/scrolled/package/spec/frontend/features/usePhonePlatform-spec.js new file mode 100644 index 0000000000..44e8fb78d0 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/features/usePhonePlatform-spec.js @@ -0,0 +1,30 @@ +import React from 'react'; + +import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {usePhonePlatform} from 'frontend/usePhonePlatform'; +import {browser} from 'pageflow/frontend'; +import '@testing-library/jest-dom/extend-expect'; + +function PhonePlatformProbe() { + return
{usePhonePlatform() ? 'phone' : 'desktop'}
; +} + +describe('usePhonePlatform', () => { + usePageObjects(); + + it('is true on a phone platform', () => { + jest.spyOn(browser, 'has').mockReturnValue(true); + + const {getByTestId} = renderEntry({contentElement: {ui: }}); + + expect(getByTestId('probe')).toHaveTextContent('phone'); + }); + + it('is false on desktop', () => { + jest.spyOn(browser, 'has').mockReturnValue(false); + + const {getByTestId} = renderEntry({contentElement: {ui: }}); + + expect(getByTestId('probe')).toHaveTextContent('desktop'); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/usePhonePlatform-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/usePhonePlatform-spec.js new file mode 100644 index 0000000000..bac4678f1d --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/usePhonePlatform-spec.js @@ -0,0 +1,42 @@ +import React from 'react'; + +import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects/inlineEditing'; +import {asyncHandlingOf} from 'support/asyncHandlingOf'; +import {usePhonePlatform} from 'frontend/usePhonePlatform'; +import '@testing-library/jest-dom/extend-expect'; + +function PhonePlatformProbe() { + return
{usePhonePlatform() ? 'phone' : 'desktop'}
; +} + +describe('inline editing usePhonePlatform', () => { + useInlineEditingPageObjects(); + + it('reflects the editor phone emulation mode', async () => { + const {getByTestId} = renderEntry({contentElement: {ui: }}); + + expect(getByTestId('probe')).toHaveTextContent('desktop'); + + await asyncHandlingOf(() => { + window.postMessage({type: 'CHANGE_EMULATION_MODE', payload: 'phone'}, '*'); + }); + + expect(getByTestId('probe')).toHaveTextContent('phone'); + }); + + it('switches back when emulation mode is reset', async () => { + const {getByTestId} = renderEntry({contentElement: {ui: }}); + + await asyncHandlingOf(() => { + window.postMessage({type: 'CHANGE_EMULATION_MODE', payload: 'phone'}, '*'); + }); + + expect(getByTestId('probe')).toHaveTextContent('phone'); + + await asyncHandlingOf(() => { + window.postMessage({type: 'CHANGE_EMULATION_MODE', payload: undefined}, '*'); + }); + + expect(getByTestId('probe')).toHaveTextContent('desktop'); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/usePhonePlatform/inEditorPreview-spec.js b/entry_types/scrolled/package/spec/frontend/usePhonePlatform/inEditorPreview-spec.js deleted file mode 100644 index bb4cf276bf..0000000000 --- a/entry_types/scrolled/package/spec/frontend/usePhonePlatform/inEditorPreview-spec.js +++ /dev/null @@ -1,38 +0,0 @@ -import {usePhonePlatform} from 'frontend/usePhonePlatform'; -import {loadInlineEditingExtensions} from 'frontend/inlineEditing'; - -import {renderHookInEntry} from 'support'; -import {asyncHandlingOf} from 'support/asyncHandlingOf'; - -import '@testing-library/jest-dom/extend-expect' - -describe('usePhonePlatform', () => { - beforeAll(loadInlineEditingExtensions); - - it('sets value when emulation mode is mobile', async () => { - const {result} = renderHookInEntry(() => usePhonePlatform()); - - await asyncHandlingOf(() => { - window.postMessage({ - type: 'CHANGE_EMULATION_MODE', - payload: 'phone' - }, '*'); - }); - - expect(result.current).toEqual(true); - }); - - it('sets value when emulation mode is desktop', async () => { - const {result} = renderHookInEntry(() => usePhonePlatform()); - - await asyncHandlingOf(() => { - window.postMessage({ - type: 'CHANGE_EMULATION_MODE', - payload: undefined - }, '*'); - }); - - expect(result.current).toEqual(false); - }); - -}); diff --git a/entry_types/scrolled/package/spec/frontend/usePhonePlatform/inFrontend-spec.js b/entry_types/scrolled/package/spec/frontend/usePhonePlatform/inFrontend-spec.js deleted file mode 100644 index 40d759d06b..0000000000 --- a/entry_types/scrolled/package/spec/frontend/usePhonePlatform/inFrontend-spec.js +++ /dev/null @@ -1,22 +0,0 @@ -import {usePhonePlatform} from 'frontend/usePhonePlatform'; - -import {browser} from 'pageflow/frontend'; -import {renderHookInEntry} from 'support'; - -describe('usePhonePlatform', () => { - it('returns true on phone platform', () => { - jest.spyOn(browser, 'has').mockReturnValue(true); - - const {result} = renderHookInEntry(() => usePhonePlatform()); - - expect(result.current).toEqual(true); - }); - - it('returns false on Desktop', () => { - jest.spyOn(browser, 'has').mockReturnValue(false); - - const {result} = renderHookInEntry(() => usePhonePlatform()); - - expect(result.current).toEqual(false); - }); -}); From 5c0a9931f8f08c8f4190c7ccacba49902e314d9e Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 13:00:25 +0200 Subject: [PATCH 06/11] Split AudioPlayer and VideoPlayer specs into features directories --- .../testing_conventions/spec-layout.md | 16 +- .../features/mediaPlayer-spec.js} | 87 +---- .../AudioPlayer/features/poster-spec.js | 51 +++ .../AudioPlayer/features/rendering-spec.js | 93 +++++ .../{ => features}/structuredData-spec.js | 0 .../package/spec/frontend/VideoPlayer-spec.js | 321 ------------------ .../features/coverPositioning-spec.js | 92 +++++ .../VideoPlayer/features/mediaPlayer-spec.js | 168 +++++++++ .../VideoPlayer/features/poster-spec.js | 71 ++++ .../VideoPlayer/features/rendering-spec.js | 94 +++++ .../{ => features}/structuredData-spec.js | 0 11 files changed, 584 insertions(+), 409 deletions(-) rename entry_types/scrolled/package/spec/frontend/{AudioPlayer-spec.js => AudioPlayer/features/mediaPlayer-spec.js} (54%) create mode 100644 entry_types/scrolled/package/spec/frontend/AudioPlayer/features/poster-spec.js create mode 100644 entry_types/scrolled/package/spec/frontend/AudioPlayer/features/rendering-spec.js rename entry_types/scrolled/package/spec/frontend/AudioPlayer/{ => features}/structuredData-spec.js (100%) delete mode 100644 entry_types/scrolled/package/spec/frontend/VideoPlayer-spec.js create mode 100644 entry_types/scrolled/package/spec/frontend/VideoPlayer/features/coverPositioning-spec.js create mode 100644 entry_types/scrolled/package/spec/frontend/VideoPlayer/features/mediaPlayer-spec.js create mode 100644 entry_types/scrolled/package/spec/frontend/VideoPlayer/features/poster-spec.js create mode 100644 entry_types/scrolled/package/spec/frontend/VideoPlayer/features/rendering-spec.js rename entry_types/scrolled/package/spec/frontend/VideoPlayer/{ => features}/structuredData-spec.js (100%) diff --git a/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md b/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md index 8f4e4a39a3..2ff058df23 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md @@ -19,14 +19,24 @@ regardless of whether the unit under test is a `.js` file or a `/` directory. This keeps spec layout invariant under `src/Foo.js → src/Foo/index.js` refactorings. +It follows that a unit's behavior tests live in exactly one place: the +flat `spec/Foo-spec.js`, or — once split — `spec/Foo/features/`, but +**never both**. A `spec/Foo-spec.js` next to a `spec/Foo/features/` +directory means a `src/Foo.js → src/Foo/index.js` refactoring left the +top-level spec behind; fold it into `features/`. (Helper and +sub-component unit specs at `spec/Foo/-spec.js` test *different* +units and may sit beside a flat `spec/Foo-spec.js` — it is specifically +`Foo-spec.js` together with `Foo/features/` that must not coexist.) + A *unit spec* mirrors a single source unit and stays at the directory level, named after that unit. A unit is usually a separate file (`spec/Foo/-spec.js` for `src/Foo/.js`), but can also be a **named export** of the main file — e.g. `AudioPlayer/index.js` exports both the `AudioPlayer` component and a `processSources` helper, -so `processSources-spec.js` mirrors that export and is *not* a topic -split. (`structuredData-spec.js` in the same directory, which exercises -the component's behavior, is.) +so `processSources-spec.js` mirrors that export and stays at the +directory level. The component's own rendered behavior (e.g. its +structured-data output) is a topic split, so it lives under `features/` +(`features/structuredData-spec.js`). `features/` holds the remaining *behavior-grouped* tests, written against the unit's stable public interface; these tend to outlive the diff --git a/entry_types/scrolled/package/spec/frontend/AudioPlayer-spec.js b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/mediaPlayer-spec.js similarity index 54% rename from entry_types/scrolled/package/spec/frontend/AudioPlayer-spec.js rename to entry_types/scrolled/package/spec/frontend/AudioPlayer/features/mediaPlayer-spec.js index 1be3d92177..4dc1fa3b55 100644 --- a/entry_types/scrolled/package/spec/frontend/AudioPlayer-spec.js +++ b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/mediaPlayer-spec.js @@ -1,14 +1,13 @@ import React from 'react'; -import '@testing-library/jest-dom/extend-expect' import 'support/mediaElementStub'; import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; -import {renderInEntry} from "../support"; +import {renderInEntry} from 'support'; import {AudioPlayer} from 'frontend/AudioPlayer'; import {useFile} from 'entryState'; import {media} from 'pageflow/frontend'; -describe('AudioPlayer', () => { +describe('AudioPlayer media player', () => { beforeEach(() => { jest.clearAllMocks(); }); @@ -40,30 +39,6 @@ describe('AudioPlayer', () => { }; } - it('renders audio tag if file is present', () => { - const result = renderInEntry( - () => , - { - seed: getAudioFileSeed({ - permaId: 100 - }) - } - ); - - expect(result.container.querySelector('audio')).toBeDefined(); - }); - - it('does not render audio element when load is "none"', () => { - const result = - renderInEntry(() => , - {seed: getAudioFileSeed()}); - - expect(result.container.querySelector('audio')).toBeNull(); - }); - it('passes correct mp3, m4a and ogg sources to media API', () => { const spyMedia = jest.spyOn(media, 'getPlayer') @@ -147,62 +122,4 @@ describe('AudioPlayer', () => { expect(spyMedia).not.toHaveBeenCalled(); }); - - it('renders given poster image', () => { - const {getByRole} = renderInEntry( - () => , - { - seed: { - fileUrlTemplates: { - audioFiles: { - mp3: ':id_partition/audio.mp3', - m4a: ':id_partition/audio.m4a', - ogg: ':id_partition/audio.ogg' - }, - imageFiles: { - large: ':id_partition/large.jpg' - } - }, - audioFiles: [ - {id: 1, permaId: 100, isReady: true} - ], - imageFiles: [ - {id: 2, permaId: 200, isReady: true} - ] - } - } - ); - - expect(getByRole('img')).toHaveAttribute('src', '000/000/002/large.jpg'); - }); - - it('renders alt text', () => { - const result = renderInEntry( - () => , - { - seed: getAudioFileSeed({ - permaId: 100, configuration: {alt: 'jingle'} - }) - } - ); - - expect(result.container.querySelector('audio')).toHaveAttribute('alt', 'jingle'); - }); - - it('renders empty alt attr', () => { - const result = renderInEntry( - () => , - { - seed: getAudioFileSeed({ - permaId: 100 - }) - } - ); - - expect(result.container.querySelector('audio').hasAttribute('alt')).toBe(true); - }); }); diff --git a/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/poster-spec.js b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/poster-spec.js new file mode 100644 index 0000000000..221548a1ee --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/poster-spec.js @@ -0,0 +1,51 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect' +import 'support/mediaElementStub'; +import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; + +import {renderInEntry} from 'support'; +import {AudioPlayer} from 'frontend/AudioPlayer'; +import {useFile} from 'entryState'; + +describe('AudioPlayer poster', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + function requiredProps() { + return { + playerState: getInitialPlayerState(), + playerActions: getPlayerActions() + }; + } + + it('renders given poster image', () => { + const {getByRole} = renderInEntry( + () => , + { + seed: { + fileUrlTemplates: { + audioFiles: { + mp3: ':id_partition/audio.mp3', + m4a: ':id_partition/audio.m4a', + ogg: ':id_partition/audio.ogg' + }, + imageFiles: { + large: ':id_partition/large.jpg' + } + }, + audioFiles: [ + {id: 1, permaId: 100, isReady: true} + ], + imageFiles: [ + {id: 2, permaId: 200, isReady: true} + ] + } + } + ); + + expect(getByRole('img')).toHaveAttribute('src', '000/000/002/large.jpg'); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/rendering-spec.js b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/rendering-spec.js new file mode 100644 index 0000000000..44db8a27bd --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/rendering-spec.js @@ -0,0 +1,93 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect' +import 'support/mediaElementStub'; +import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; + +import {renderInEntry} from 'support'; +import {AudioPlayer} from 'frontend/AudioPlayer'; +import {useFile} from 'entryState'; + +describe('AudioPlayer rendering', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + function getAudioFileSeed({ + id = 1, + permaId = 100, + basename = 'audio', + configuration = {} + } = {}) { + return { + fileUrlTemplates: { + audioFiles: { + mp3: ':id_partition/:basename.mp3', + m4a: ':id_partition/:basename.m4a', + ogg: ':id_partition/:basename.ogg' + } + }, + audioFiles: [ + {id, permaId, isReady: true, basename, configuration} + ] + }; + } + + function requiredProps() { + return { + playerState: getInitialPlayerState(), + playerActions: getPlayerActions() + }; + } + + it('renders audio tag if file is present', () => { + const result = renderInEntry( + () => , + { + seed: getAudioFileSeed({ + permaId: 100 + }) + } + ); + + expect(result.container.querySelector('audio')).toBeDefined(); + }); + + it('does not render audio element when load is "none"', () => { + const result = + renderInEntry(() => , + {seed: getAudioFileSeed()}); + + expect(result.container.querySelector('audio')).toBeNull(); + }); + + it('renders alt text', () => { + const result = renderInEntry( + () => , + { + seed: getAudioFileSeed({ + permaId: 100, configuration: {alt: 'jingle'} + }) + } + ); + + expect(result.container.querySelector('audio')).toHaveAttribute('alt', 'jingle'); + }); + + it('renders empty alt attr', () => { + const result = renderInEntry( + () => , + { + seed: getAudioFileSeed({ + permaId: 100 + }) + } + ); + + expect(result.container.querySelector('audio').hasAttribute('alt')).toBe(true); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/AudioPlayer/structuredData-spec.js b/entry_types/scrolled/package/spec/frontend/AudioPlayer/features/structuredData-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/frontend/AudioPlayer/structuredData-spec.js rename to entry_types/scrolled/package/spec/frontend/AudioPlayer/features/structuredData-spec.js diff --git a/entry_types/scrolled/package/spec/frontend/VideoPlayer-spec.js b/entry_types/scrolled/package/spec/frontend/VideoPlayer-spec.js deleted file mode 100644 index 15f5fc0b0a..0000000000 --- a/entry_types/scrolled/package/spec/frontend/VideoPlayer-spec.js +++ /dev/null @@ -1,321 +0,0 @@ -import React from 'react'; -import '@testing-library/jest-dom/extend-expect' -import 'support/mediaElementStub'; -import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; - -import {renderInEntry} from "../support"; -import {useBackgroundFile} from 'frontend/useBackgroundFile'; -import {useFile} from 'entryState'; -import {VideoPlayer} from 'frontend/VideoPlayer'; -import {media, settings} from 'pageflow/frontend'; - -describe('VideoPlayer', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - function getVideoFileSeed({ - id = 1, - permaId = 100, - basename = 'video', - configuration = {} - } = {}) { - return { - fileUrlTemplates: { - videoFiles: { - medium: ':id_partition/medium/:basename.mp4', - high: ':id_partition/high/:basename.mp4', - 'hls-playlist': ':id_partition/hls-playlist.m3u8', - 'hls-playlist-high-and-up': ':id_partition/hls-playlist-high-and-up.m3u8' - } - }, - videoFiles: [ - {id, permaId, isReady: true, basename, configuration} - ] - }; - } - - function requiredProps() { - return { - playerState: getInitialPlayerState(), - playerActions: getPlayerActions() - }; - } - - it('renders video with provided file id', () => { - const result = renderInEntry( - () => , - {seed: getVideoFileSeed({permaId: 100})} - ); - - expect(result.container.querySelector('video')).toBeDefined(); - }); - - it('does not render video element when load is "none"', () => { - const result = renderInEntry( - () => , - {seed: getVideoFileSeed()} - ); - - expect(result.container.querySelector('video')).toBeNull(); - }); - - it('renders null when file is undefined and fit is cover', () => { - const result = - renderInEntry(, - {seed: getVideoFileSeed()}); - - expect(result.container.querySelector('video')).toBeNull(); - }); - - it('passes sources according to setting to media API', () => { - const spyMedia = jest.spyOn(media, 'getPlayer'); - settings.set('videoQuality', 'auto'); - - renderInEntry( - () => , - { - seed: getVideoFileSeed({ - basename: 'video', - id: 1, - permaId: 100 - }) - } - ); - - expect(spyMedia).toHaveBeenCalledWith( - [{type: 'application/x-mpegURL', src: '000/000/001/hls-playlist.m3u8'}, - {type: 'video/mp4', src: '000/000/001/high/video.mp4'}], - expect.anything() - ); - }); - - it('support adaptiveMinQuality prop', () => { - const spyMedia = jest.spyOn(media, 'getPlayer'); - settings.set('videoQuality', 'auto'); - - renderInEntry( - () => , - { - seed: getVideoFileSeed({ - basename: 'video', - id: 1, - permaId: 100 - }) - } - ); - - expect(spyMedia).toHaveBeenCalledWith( - [{type: 'application/x-mpegURL', src: '000/000/001/hls-playlist-high-and-up.m3u8'}, - {type: 'video/mp4', src: '000/000/001/high/video.mp4'}], - expect.anything() - ); - }); - - it('uses quality from settings', () => { - const spyMedia = jest.spyOn(media, 'getPlayer'); - settings.set('videoQuality', 'medium'); - - renderInEntry( - () => , - { - seed: getVideoFileSeed({ - basename: 'video', - id: 1, - permaId: 100 - }) - } - ); - - expect(spyMedia).toHaveBeenCalledWith( - [{type: 'video/mp4', src: '000/000/001/medium/video.mp4'}], - expect.anything() - ); - }); - - it('passes file perma id to media api', () => { - const spyMedia = jest.spyOn(media, 'getPlayer'); - - renderInEntry( - () => , - { - seed: getVideoFileSeed({ - basename: 'video', - id: 1, - permaId: 100 - }) - } - ); - - expect(spyMedia).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({filePermaId: 100}) - ); - }); - - it('passes displayName from video file as media events context data', () => { - const spyMedia = jest.spyOn(media, 'getPlayer'); - - renderInEntry( - () => , - { - seed: { - fileUrlTemplates: { - videoFiles: { - high: ':id_partition/high/:basename.mp4' - } - }, - videoFiles: [ - {id: 1, permaId: 100, isReady: true, displayName: 'Interview.mp4'} - ] - } - } - ); - - expect(spyMedia).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - mediaEventsContextData: expect.objectContaining({ - fileDisplayName: 'Interview.mp4' - }) - }) - ); - }); - - it('without id no media player is request', () => { - const spyMedia = jest.spyOn(media, 'getPlayer'); - renderInEntry(); - expect(spyMedia).not.toHaveBeenCalled(); - }); - - it('renders given poster image', () => { - const {getByRole} = renderInEntry( - () => , - { - seed: { - fileUrlTemplates: { - videoFiles: { - high: ':id_partition/video.mp4' - }, - imageFiles: { - large: ':id_partition/large.jpg' - } - }, - videoFiles: [ - {id: 1, permaId: 100, isReady: true} - ], - imageFiles: [ - {id: 2, permaId: 200, isReady: true} - ] - } - } - ); - - expect(getByRole('img')).toHaveAttribute('src', '000/000/002/large.jpg'); - }); - - it('falls back to video poster', () => { - const {getByRole} = renderInEntry( - () => , - { - seed: { - fileUrlTemplates: { - videoFiles: { - high: ':id_partition/video.mp4', - posterLarge: ':id_partition/poster.jpg' - } - }, - videoFiles: [ - {id: 1, permaId: 100, isReady: true} - ] - } - } - ); - - expect(getByRole('img')).toHaveAttribute('src', '000/000/001/poster.jpg'); - }); - - it('renders alt text', () => { - const result = renderInEntry( - () => , - { - seed: getVideoFileSeed({permaId: 100, configuration: {alt: 'interview'}}) - } - ); - - expect(result.container.querySelector('video')).toHaveAttribute('alt', 'interview'); - }); - - it('renders empty alt attr', () => { - const result = renderInEntry( - () => , - { - seed: getVideoFileSeed({permaId: 100}) - }); - - expect(result.container.querySelector('video').hasAttribute('alt')).toBe(true); - }); - - it('sets object position based on motif area to media api when fit is cover', () => { - const result = renderInEntry( - () => { - const file = useBackgroundFile({ - file: useFile({collectionName: 'videoFiles', permaId: 100}), - motifArea: {left: 50, top: 0, width: 50, height: 40}, - containerDimension: {width: 1000, height: 1000} - }); - - return ( - - ); - }, - { - seed: getVideoFileSeed({ - permaId: 100, width: 2000, height: 1000 - }) - } - ); - - expect(result.container.querySelector('video')).toHaveStyle('object-position: 100% 50%'); - }); - - it('does not set object position when fit is not cover', () => { - const result = renderInEntry( - () => { - const file = useBackgroundFile({ - file: useFile({collectionName: 'videoFiles', permaId: 100}), - motifArea: {left: 50, top: 0, width: 50, height: 40}, - containerDimension: {width: 1000, height: 1000} - }); - - return ( - - ); - }, - { - seed: getVideoFileSeed({ - permaId: 100, width: 2000, height: 1000 - }) - } - ); - - expect(result.container.querySelector('video')).toHaveAttribute('style', ''); - }); -}); diff --git a/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/coverPositioning-spec.js b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/coverPositioning-spec.js new file mode 100644 index 0000000000..f19577521c --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/coverPositioning-spec.js @@ -0,0 +1,92 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect' +import 'support/mediaElementStub'; +import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; + +import {renderInEntry} from 'support'; +import {useBackgroundFile} from 'frontend/useBackgroundFile'; +import {useFile} from 'entryState'; +import {VideoPlayer} from 'frontend/VideoPlayer'; + +describe('VideoPlayer cover positioning', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + function getVideoFileSeed({ + id = 1, + permaId = 100, + basename = 'video', + configuration = {} + } = {}) { + return { + fileUrlTemplates: { + videoFiles: { + medium: ':id_partition/medium/:basename.mp4', + high: ':id_partition/high/:basename.mp4', + 'hls-playlist': ':id_partition/hls-playlist.m3u8', + 'hls-playlist-high-and-up': ':id_partition/hls-playlist-high-and-up.m3u8' + } + }, + videoFiles: [ + {id, permaId, isReady: true, basename, configuration} + ] + }; + } + + function requiredProps() { + return { + playerState: getInitialPlayerState(), + playerActions: getPlayerActions() + }; + } + + it('sets object position based on motif area to media api when fit is cover', () => { + const result = renderInEntry( + () => { + const file = useBackgroundFile({ + file: useFile({collectionName: 'videoFiles', permaId: 100}), + motifArea: {left: 50, top: 0, width: 50, height: 40}, + containerDimension: {width: 1000, height: 1000} + }); + + return ( + + ); + }, + { + seed: getVideoFileSeed({ + permaId: 100, width: 2000, height: 1000 + }) + } + ); + + expect(result.container.querySelector('video')).toHaveStyle('object-position: 100% 50%'); + }); + + it('does not set object position when fit is not cover', () => { + const result = renderInEntry( + () => { + const file = useBackgroundFile({ + file: useFile({collectionName: 'videoFiles', permaId: 100}), + motifArea: {left: 50, top: 0, width: 50, height: 40}, + containerDimension: {width: 1000, height: 1000} + }); + + return ( + + ); + }, + { + seed: getVideoFileSeed({ + permaId: 100, width: 2000, height: 1000 + }) + } + ); + + expect(result.container.querySelector('video')).toHaveAttribute('style', ''); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/mediaPlayer-spec.js b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/mediaPlayer-spec.js new file mode 100644 index 0000000000..84043832b9 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/mediaPlayer-spec.js @@ -0,0 +1,168 @@ +import React from 'react'; +import 'support/mediaElementStub'; +import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; + +import {renderInEntry} from 'support'; +import {useFile} from 'entryState'; +import {VideoPlayer} from 'frontend/VideoPlayer'; +import {media, settings} from 'pageflow/frontend'; + +describe('VideoPlayer media player', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + function getVideoFileSeed({ + id = 1, + permaId = 100, + basename = 'video', + configuration = {} + } = {}) { + return { + fileUrlTemplates: { + videoFiles: { + medium: ':id_partition/medium/:basename.mp4', + high: ':id_partition/high/:basename.mp4', + 'hls-playlist': ':id_partition/hls-playlist.m3u8', + 'hls-playlist-high-and-up': ':id_partition/hls-playlist-high-and-up.m3u8' + } + }, + videoFiles: [ + {id, permaId, isReady: true, basename, configuration} + ] + }; + } + + function requiredProps() { + return { + playerState: getInitialPlayerState(), + playerActions: getPlayerActions() + }; + } + + it('passes sources according to setting to media API', () => { + const spyMedia = jest.spyOn(media, 'getPlayer'); + settings.set('videoQuality', 'auto'); + + renderInEntry( + () => , + { + seed: getVideoFileSeed({ + basename: 'video', + id: 1, + permaId: 100 + }) + } + ); + + expect(spyMedia).toHaveBeenCalledWith( + [{type: 'application/x-mpegURL', src: '000/000/001/hls-playlist.m3u8'}, + {type: 'video/mp4', src: '000/000/001/high/video.mp4'}], + expect.anything() + ); + }); + + it('support adaptiveMinQuality prop', () => { + const spyMedia = jest.spyOn(media, 'getPlayer'); + settings.set('videoQuality', 'auto'); + + renderInEntry( + () => , + { + seed: getVideoFileSeed({ + basename: 'video', + id: 1, + permaId: 100 + }) + } + ); + + expect(spyMedia).toHaveBeenCalledWith( + [{type: 'application/x-mpegURL', src: '000/000/001/hls-playlist-high-and-up.m3u8'}, + {type: 'video/mp4', src: '000/000/001/high/video.mp4'}], + expect.anything() + ); + }); + + it('uses quality from settings', () => { + const spyMedia = jest.spyOn(media, 'getPlayer'); + settings.set('videoQuality', 'medium'); + + renderInEntry( + () => , + { + seed: getVideoFileSeed({ + basename: 'video', + id: 1, + permaId: 100 + }) + } + ); + + expect(spyMedia).toHaveBeenCalledWith( + [{type: 'video/mp4', src: '000/000/001/medium/video.mp4'}], + expect.anything() + ); + }); + + it('passes file perma id to media api', () => { + const spyMedia = jest.spyOn(media, 'getPlayer'); + + renderInEntry( + () => , + { + seed: getVideoFileSeed({ + basename: 'video', + id: 1, + permaId: 100 + }) + } + ); + + expect(spyMedia).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({filePermaId: 100}) + ); + }); + + it('passes displayName from video file as media events context data', () => { + const spyMedia = jest.spyOn(media, 'getPlayer'); + + renderInEntry( + () => , + { + seed: { + fileUrlTemplates: { + videoFiles: { + high: ':id_partition/high/:basename.mp4' + } + }, + videoFiles: [ + {id: 1, permaId: 100, isReady: true, displayName: 'Interview.mp4'} + ] + } + } + ); + + expect(spyMedia).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + mediaEventsContextData: expect.objectContaining({ + fileDisplayName: 'Interview.mp4' + }) + }) + ); + }); + + it('without id no media player is request', () => { + const spyMedia = jest.spyOn(media, 'getPlayer'); + renderInEntry(); + expect(spyMedia).not.toHaveBeenCalled(); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/poster-spec.js b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/poster-spec.js new file mode 100644 index 0000000000..4e9a90911c --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/poster-spec.js @@ -0,0 +1,71 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect' +import 'support/mediaElementStub'; +import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; + +import {renderInEntry} from 'support'; +import {useFile} from 'entryState'; +import {VideoPlayer} from 'frontend/VideoPlayer'; + +describe('VideoPlayer poster', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + function requiredProps() { + return { + playerState: getInitialPlayerState(), + playerActions: getPlayerActions() + }; + } + + it('renders given poster image', () => { + const {getByRole} = renderInEntry( + () => , + { + seed: { + fileUrlTemplates: { + videoFiles: { + high: ':id_partition/video.mp4' + }, + imageFiles: { + large: ':id_partition/large.jpg' + } + }, + videoFiles: [ + {id: 1, permaId: 100, isReady: true} + ], + imageFiles: [ + {id: 2, permaId: 200, isReady: true} + ] + } + } + ); + + expect(getByRole('img')).toHaveAttribute('src', '000/000/002/large.jpg'); + }); + + it('falls back to video poster', () => { + const {getByRole} = renderInEntry( + () => , + { + seed: { + fileUrlTemplates: { + videoFiles: { + high: ':id_partition/video.mp4', + posterLarge: ':id_partition/poster.jpg' + } + }, + videoFiles: [ + {id: 1, permaId: 100, isReady: true} + ] + } + } + ); + + expect(getByRole('img')).toHaveAttribute('src', '000/000/001/poster.jpg'); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/rendering-spec.js b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/rendering-spec.js new file mode 100644 index 0000000000..33e441a12f --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/rendering-spec.js @@ -0,0 +1,94 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect' +import 'support/mediaElementStub'; +import {getInitialPlayerState, getPlayerActions} from 'support/fakePlayerState'; + +import {renderInEntry} from 'support'; +import {useFile} from 'entryState'; +import {VideoPlayer} from 'frontend/VideoPlayer'; + +describe('VideoPlayer rendering', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + function getVideoFileSeed({ + id = 1, + permaId = 100, + basename = 'video', + configuration = {} + } = {}) { + return { + fileUrlTemplates: { + videoFiles: { + medium: ':id_partition/medium/:basename.mp4', + high: ':id_partition/high/:basename.mp4', + 'hls-playlist': ':id_partition/hls-playlist.m3u8', + 'hls-playlist-high-and-up': ':id_partition/hls-playlist-high-and-up.m3u8' + } + }, + videoFiles: [ + {id, permaId, isReady: true, basename, configuration} + ] + }; + } + + function requiredProps() { + return { + playerState: getInitialPlayerState(), + playerActions: getPlayerActions() + }; + } + + it('renders video with provided file id', () => { + const result = renderInEntry( + () => , + {seed: getVideoFileSeed({permaId: 100})} + ); + + expect(result.container.querySelector('video')).toBeDefined(); + }); + + it('does not render video element when load is "none"', () => { + const result = renderInEntry( + () => , + {seed: getVideoFileSeed()} + ); + + expect(result.container.querySelector('video')).toBeNull(); + }); + + it('renders null when file is undefined and fit is cover', () => { + const result = + renderInEntry(, + {seed: getVideoFileSeed()}); + + expect(result.container.querySelector('video')).toBeNull(); + }); + + it('renders alt text', () => { + const result = renderInEntry( + () => , + { + seed: getVideoFileSeed({permaId: 100, configuration: {alt: 'interview'}}) + } + ); + + expect(result.container.querySelector('video')).toHaveAttribute('alt', 'interview'); + }); + + it('renders empty alt attr', () => { + const result = renderInEntry( + () => , + { + seed: getVideoFileSeed({permaId: 100}) + }); + + expect(result.container.querySelector('video').hasAttribute('alt')).toBe(true); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/VideoPlayer/structuredData-spec.js b/entry_types/scrolled/package/spec/frontend/VideoPlayer/features/structuredData-spec.js similarity index 100% rename from entry_types/scrolled/package/spec/frontend/VideoPlayer/structuredData-spec.js rename to entry_types/scrolled/package/spec/frontend/VideoPlayer/features/structuredData-spec.js From bbcae72596f9a52b5fc0a51e87f980aba5f2c942 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 20:47:08 +0200 Subject: [PATCH 07/11] Extract section page object predicates into matchers --- .../internal/testing_conventions/matchers.md | 17 ++++++- .../testing_conventions/render-helpers.md | 3 +- .../features/constrainContentWidth-spec.js | 10 +++-- .../frontend/features/perElementFade-spec.js | 6 ++- .../frontend/features/sectionPadding-spec.js | 25 +++++------ .../frontend/features/transitions-spec.js | 14 +++--- .../features/forcedSectionPadding-spec.js | 8 ++-- .../package/spec/support/matchers/index.js | 22 ++++++++++ .../matchers/toHaveConstrainedContentWidth.js | 19 ++++++++ .../matchers/toHaveFadedOutForeground.js | 16 +++++++ .../toHaveFirstBoxSuppressedTopMargin.js | 16 +++++++ .../support/matchers/toHaveForcedPadding.js | 15 +++++++ .../toHavePerElementFadeTransition.js | 16 +++++++ .../support/matchers/toHaveRemainingSpace.js | 32 ++++++++++++++ .../matchers/toHaveSuppressedPadding.js | 32 ++++++++++++++ .../package/spec/support/pageObjects/index.js | 44 ------------------- 16 files changed, 221 insertions(+), 74 deletions(-) create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveConstrainedContentWidth.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveFadedOutForeground.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveFirstBoxSuppressedTopMargin.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveForcedPadding.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHavePerElementFadeTransition.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveRemainingSpace.js create mode 100644 entry_types/scrolled/package/spec/support/matchers/toHaveSuppressedPadding.js diff --git a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md index 2c2b297d0f..2651da42c5 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md @@ -50,7 +50,22 @@ render these wrappers. | `toHaveScrollSpace()` | The element sits inside a scroll-space wrapper. | | `toHaveAlignment(value)` | The element carries the alignment class for the given value. | -Both sets are registered through the `useXxx` hooks listed under +**Section matchers** (`spec/support/matchers/`, enabled with +`useSectionMatchers()`) assert a section's foreground/layout state. The +subject is a section page object (`getSectionByPermaId(...)`), so they +also require `renderEntry`. + +| Matcher | Asserts | +| --- | --- | +| `toHaveSuppressedPadding({top, bottom})` | The foreground suppresses top/bottom padding. | +| `toHaveRemainingSpace({above, below})` | The foreground keeps remaining vertical space above/below the content. | +| `toHaveForcedPadding()` | The foreground forces padding (inline editing). | +| `toHaveFadedOutForeground()` | The foreground is faded out by a transition. | +| `toHavePerElementFadeTransition()` | The section uses the per-element fade transition. | +| `toHaveFirstBoxSuppressedTopMargin()` | The first foreground box has its top margin suppressed. | +| `toHaveConstrainedContentWidth()` | The section constrains its content width. | + +These sets are registered through the `useXxx` hooks listed under [Setup hooks](render-helpers.md#setup-hooks). ## Adding a matcher diff --git a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md index fe43c1866d..2fa1d5796c 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md @@ -67,7 +67,8 @@ in the order they appear below. | `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 layout [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 diff --git a/entry_types/scrolled/package/spec/frontend/features/constrainContentWidth-spec.js b/entry_types/scrolled/package/spec/frontend/features/constrainContentWidth-spec.js index 66396fc0ad..87741be95d 100644 --- a/entry_types/scrolled/package/spec/frontend/features/constrainContentWidth-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/constrainContentWidth-spec.js @@ -1,5 +1,6 @@ import React from 'react'; import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useSectionMatchers} from 'support/matchers'; import '@testing-library/jest-dom/extend-expect'; import {api} from 'frontend/api'; @@ -9,6 +10,7 @@ jest.mock('frontend/useMotifAreaState'); describe('constrainContentWidth', () => { usePageObjects(); + useSectionMatchers(); beforeEach(() => { api.contentElementTypes.register('probe', { @@ -31,7 +33,7 @@ describe('constrainContentWidth', () => { } }); - expect(getSectionByPermaId(6).hasConstrainedContentWidth()).toBe(true); + expect(getSectionByPermaId(6)).toHaveConstrainedContentWidth(); }); it('does not apply class for shadow appearance', () => { @@ -44,7 +46,7 @@ describe('constrainContentWidth', () => { } }); - expect(getSectionByPermaId(6).hasConstrainedContentWidth()).toBe(false); + expect(getSectionByPermaId(6)).not.toHaveConstrainedContentWidth(); }); it('applies class for split appearance with center layout', () => { @@ -59,7 +61,7 @@ describe('constrainContentWidth', () => { } }); - expect(getSectionByPermaId(6).hasConstrainedContentWidth()).toBe(true); + expect(getSectionByPermaId(6)).toHaveConstrainedContentWidth(); }); it('does not apply class when content is padded', () => { @@ -74,7 +76,7 @@ describe('constrainContentWidth', () => { } }); - expect(getSectionByPermaId(6).hasConstrainedContentWidth()).toBe(false); + expect(getSectionByPermaId(6)).not.toHaveConstrainedContentWidth(); }); it('passes constrainContentWidth to content elements via sectionProps', () => { diff --git a/entry_types/scrolled/package/spec/frontend/features/perElementFade-spec.js b/entry_types/scrolled/package/spec/frontend/features/perElementFade-spec.js index acf0caa257..a3099a821e 100644 --- a/entry_types/scrolled/package/spec/frontend/features/perElementFade-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/perElementFade-spec.js @@ -1,7 +1,9 @@ import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useSectionMatchers} from 'support/matchers'; describe('fade transitions with backdrop blur', () => { usePageObjects(); + useSectionMatchers(); it('uses per-element fade to preserve backdrop blur', () => { const {getSectionByPermaId} = renderEntry({ @@ -16,7 +18,7 @@ describe('fade transitions with backdrop blur', () => { } }); - expect(getSectionByPermaId(10).usesPerElementFadeTransition()).toBe(true); + expect(getSectionByPermaId(10)).toHavePerElementFadeTransition(); }); it('uses regular fade when there is no backdrop blur', () => { @@ -32,6 +34,6 @@ describe('fade transitions with backdrop blur', () => { } }); - expect(getSectionByPermaId(10).usesPerElementFadeTransition()).toBe(false); + expect(getSectionByPermaId(10)).not.toHavePerElementFadeTransition(); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/sectionPadding-spec.js b/entry_types/scrolled/package/spec/frontend/features/sectionPadding-spec.js index 9db4c5b6c2..b87b406169 100644 --- a/entry_types/scrolled/package/spec/frontend/features/sectionPadding-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/sectionPadding-spec.js @@ -1,4 +1,5 @@ import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useSectionMatchers} from 'support/matchers'; import {act} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; @@ -10,6 +11,7 @@ jest.mock('frontend/useMotifAreaState'); describe('section padding', () => { usePageObjects(); + useSectionMatchers(); it('does not suppress top padding by default', () => { const {getSectionByPermaId} = renderEntry({ @@ -19,7 +21,7 @@ describe('section padding', () => { } }); - expect(getSectionByPermaId(6).hasSuppressedTopPadding()).toBe(false); + expect(getSectionByPermaId(6)).toHaveSuppressedPadding({top: false}); }); it('does not suppress bottom padding by default', () => { @@ -30,7 +32,7 @@ describe('section padding', () => { } }); - expect(getSectionByPermaId(6).hasSuppressedBottomPadding()).toBe(false); + expect(getSectionByPermaId(6)).toHaveSuppressedPadding({bottom: false}); }); it('suppresses top padding if first content element is full width', () => { @@ -41,7 +43,7 @@ describe('section padding', () => { } }); - expect(getSectionByPermaId(6).hasSuppressedTopPadding()).toBe(true); + expect(getSectionByPermaId(6)).toHaveSuppressedPadding({top: true}); }); it('suppresses top padding if motif area is content padded', () => { @@ -54,7 +56,7 @@ describe('section padding', () => { } }); - expect(getSectionByPermaId(6).hasSuppressedTopPadding()).toBe(true); + expect(getSectionByPermaId(6)).toHaveSuppressedPadding({top: true}); }); it('does not suppress first box top margin if motif area becomes content padded', () => { @@ -65,11 +67,11 @@ describe('section padding', () => { } }); - expect(getSectionByPermaId(6).hasFirstBoxSuppressedTopMargin()).toBe(true); + expect(getSectionByPermaId(6)).toHaveFirstBoxSuppressedTopMargin(); act(() => useMotifAreaState.mockContentPadded()); - expect(getSectionByPermaId(6).hasFirstBoxSuppressedTopMargin()).toBe(false); + expect(getSectionByPermaId(6)).not.toHaveFirstBoxSuppressedTopMargin(); }); it('suppresses bottom padding if last content element is full width', () => { @@ -80,7 +82,7 @@ describe('section padding', () => { } }); - expect(getSectionByPermaId(6).hasSuppressedBottomPadding()).toBe(true); + expect(getSectionByPermaId(6)).toHaveSuppressedPadding({bottom: true}); }); it('does not set inline padding styles when no paddingTop/paddingBottom set', () => { @@ -234,8 +236,7 @@ describe('section padding', () => { }); const section = getSectionByPermaId(6); - expect(section.hasRemainingSpaceAbove()).toBe(false); - expect(section.hasRemainingSpaceBelow()).toBe(false); + expect(section).toHaveRemainingSpace({above: false, below: false}); }); it('supports remaining vertical space above content', () => { @@ -251,8 +252,7 @@ describe('section padding', () => { }); const section = getSectionByPermaId(6); - expect(section.hasRemainingSpaceAbove()).toBe(true); - expect(section.hasRemainingSpaceBelow()).toBe(false); + expect(section).toHaveRemainingSpace({above: true, below: false}); }); it('supports remaining vertical space below content', () => { @@ -268,7 +268,6 @@ describe('section padding', () => { }); const section = getSectionByPermaId(6); - expect(section.hasRemainingSpaceAbove()).toBe(false); - expect(section.hasRemainingSpaceBelow()).toBe(true); + expect(section).toHaveRemainingSpace({above: false, below: true}); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/transitions-spec.js b/entry_types/scrolled/package/spec/frontend/features/transitions-spec.js index 04678d1d56..8c9917a9b3 100644 --- a/entry_types/scrolled/package/spec/frontend/features/transitions-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/transitions-spec.js @@ -1,6 +1,7 @@ import 'widgets/excursionSheet'; import {renderEntry, usePageObjects} from 'support/pageObjects'; +import {useSectionMatchers} from 'support/matchers'; import {changeLocationHash} from 'support/changeLocationHash'; import 'support/viewTimelineStub'; import 'support/animateStub'; @@ -9,6 +10,7 @@ import {act} from '@testing-library/react'; describe('transitions', () => { usePageObjects(); + useSectionMatchers(); it('applies foreground-above and foreground-below classes when scrolling middle section into view', () => { const {getSectionByPermaId} = renderEntry({ @@ -41,9 +43,9 @@ describe('transitions', () => { getSectionByPermaId(2).simulateScrollingIntoView(); - expect(getSectionByPermaId(1).hasFadedOutForeground()).toBe(true); - expect(getSectionByPermaId(2).hasFadedOutForeground()).toBe(false); - expect(getSectionByPermaId(3).hasFadedOutForeground()).toBe(true); + expect(getSectionByPermaId(1)).toHaveFadedOutForeground(); + expect(getSectionByPermaId(2)).not.toHaveFadedOutForeground(); + expect(getSectionByPermaId(3)).toHaveFadedOutForeground(); }); it('fades foreground for sections with fade transition in excursions', () => { @@ -104,8 +106,8 @@ describe('transitions', () => { act(() => changeLocationHash('#some-excursion')); getSectionByPermaId(2).simulateScrollingIntoView(); - expect(getSectionByPermaId(1).hasFadedOutForeground()).toBe(true); - expect(getSectionByPermaId(2).hasFadedOutForeground()).toBe(false); - expect(getSectionByPermaId(3).hasFadedOutForeground()).toBe(true); + expect(getSectionByPermaId(1)).toHaveFadedOutForeground(); + expect(getSectionByPermaId(2)).not.toHaveFadedOutForeground(); + expect(getSectionByPermaId(3)).toHaveFadedOutForeground(); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js index d7de9de9b2..b8fda7bb63 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js @@ -1,9 +1,11 @@ import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; +import {useSectionMatchers} from 'support/matchers'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing forced section padding', () => { useInlineEditingPageObjects(); + useSectionMatchers(); it('forces padding below full width element if section is selected', () => { const {getSectionByPermaId} = renderEntry({ @@ -16,7 +18,7 @@ describe('inline editing forced section padding', () => { const section = getSectionByPermaId(6); section.select(); - expect(section.hasForcedPadding()).toBe(true); + expect(section).toHaveForcedPadding(); }); it('forces padding below full width element if element is selected', () => { @@ -33,7 +35,7 @@ describe('inline editing forced section padding', () => { getContentElementByTestId(10).select(); - expect(getSectionByPermaId(6).hasForcedPadding()).toBe(true); + expect(getSectionByPermaId(6)).toHaveForcedPadding(); }); it('does not force padding if padding is selected', () => { @@ -47,6 +49,6 @@ describe('inline editing forced section padding', () => { const section = getSectionByPermaId(6); section.selectPadding('bottom'); - expect(section.hasForcedPadding()).toBe(false); + expect(section).not.toHaveForcedPadding(); }); }); diff --git a/entry_types/scrolled/package/spec/support/matchers/index.js b/entry_types/scrolled/package/spec/support/matchers/index.js index 81a988edae..83babf6f60 100644 --- a/entry_types/scrolled/package/spec/support/matchers/index.js +++ b/entry_types/scrolled/package/spec/support/matchers/index.js @@ -2,6 +2,14 @@ import {toHaveContentElementMargin} from './toHaveContentElementMargin'; import {toHaveScrollSpace} from './toHaveScrollSpace'; import {toHaveAlignment} from './toHaveAlignment'; +import {toHaveSuppressedPadding} from './toHaveSuppressedPadding'; +import {toHaveRemainingSpace} from './toHaveRemainingSpace'; +import {toHaveForcedPadding} from './toHaveForcedPadding'; +import {toHaveFadedOutForeground} from './toHaveFadedOutForeground'; +import {toHavePerElementFadeTransition} from './toHavePerElementFadeTransition'; +import {toHaveFirstBoxSuppressedTopMargin} from './toHaveFirstBoxSuppressedTopMargin'; +import {toHaveConstrainedContentWidth} from './toHaveConstrainedContentWidth'; + export function useContentElementLayoutMatchers() { beforeEach(() => { expect.extend({ @@ -11,3 +19,17 @@ export function useContentElementLayoutMatchers() { }); }); } + +export function useSectionMatchers() { + beforeEach(() => { + expect.extend({ + toHaveSuppressedPadding, + toHaveRemainingSpace, + toHaveForcedPadding, + toHaveFadedOutForeground, + toHavePerElementFadeTransition, + toHaveFirstBoxSuppressedTopMargin, + toHaveConstrainedContentWidth + }); + }); +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveConstrainedContentWidth.js b/entry_types/scrolled/package/spec/support/matchers/toHaveConstrainedContentWidth.js new file mode 100644 index 0000000000..8b2e15a4a4 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveConstrainedContentWidth.js @@ -0,0 +1,19 @@ +import centerLayoutStyles from 'frontend/layouts/Center.module.css'; +import twoColumnLayoutStyles from 'frontend/layouts/TwoColumn.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveConstrainedContentWidth(subject) { + const el = getElement(subject); + const pass = !!( + el.querySelector(`.${twoColumnLayoutStyles.constrainContentWidth}`) || + el.querySelector(`.${centerLayoutStyles.constrainContentWidth}`) + ); + + return { + pass, + message: () => pass + ? 'expected section not to constrain content width' + : 'expected section to constrain content width' + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveFadedOutForeground.js b/entry_types/scrolled/package/spec/support/matchers/toHaveFadedOutForeground.js new file mode 100644 index 0000000000..55b12fcf19 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveFadedOutForeground.js @@ -0,0 +1,16 @@ +import foregroundStyles from 'frontend/Foreground.module.css'; +import transitionStyles from 'frontend/transitions/shared.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveFadedOutForeground(subject) { + const foreground = getElement(subject).querySelector(`.${foregroundStyles.Foreground}`); + const pass = foreground.classList.contains(transitionStyles.fadedOut); + + return { + pass, + message: () => pass + ? 'expected section foreground not to be faded out' + : 'expected section foreground to be faded out' + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveFirstBoxSuppressedTopMargin.js b/entry_types/scrolled/package/spec/support/matchers/toHaveFirstBoxSuppressedTopMargin.js new file mode 100644 index 0000000000..2e5e9636d3 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveFirstBoxSuppressedTopMargin.js @@ -0,0 +1,16 @@ +import foregroundStyles from 'frontend/Foreground.module.css'; +import boxBoundaryMarginStyles from 'frontend/foregroundBoxes/BoxBoundaryMargin.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveFirstBoxSuppressedTopMargin(subject) { + const foreground = getElement(subject).querySelector(`.${foregroundStyles.Foreground}`); + const pass = !!foreground.querySelector(`.${boxBoundaryMarginStyles.noTopMargin}`); + + return { + pass, + message: () => pass + ? 'expected first box not to have a suppressed top margin' + : 'expected first box to have a suppressed top margin' + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveForcedPadding.js b/entry_types/scrolled/package/spec/support/matchers/toHaveForcedPadding.js new file mode 100644 index 0000000000..76116a81a3 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveForcedPadding.js @@ -0,0 +1,15 @@ +import styles from 'frontend/Foreground.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHaveForcedPadding(subject) { + const foreground = getElement(subject).querySelector(`.${styles.Foreground}`); + const pass = foreground.classList.contains(styles.forcePadding); + + return { + pass, + message: () => pass + ? 'expected section not to have forced padding' + : 'expected section to have forced padding' + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHavePerElementFadeTransition.js b/entry_types/scrolled/package/spec/support/matchers/toHavePerElementFadeTransition.js new file mode 100644 index 0000000000..ae339dfeff --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHavePerElementFadeTransition.js @@ -0,0 +1,16 @@ +import foregroundStyles from 'frontend/Foreground.module.css'; +import transitionStyles from 'frontend/transitions/shared.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +export function toHavePerElementFadeTransition(subject) { + const foreground = getElement(subject).querySelector(`.${foregroundStyles.Foreground}`); + const pass = foreground.classList.contains(transitionStyles.perElementFade); + + return { + pass, + message: () => pass + ? 'expected section not to use the per-element fade transition' + : 'expected section to use the per-element fade transition' + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveRemainingSpace.js b/entry_types/scrolled/package/spec/support/matchers/toHaveRemainingSpace.js new file mode 100644 index 0000000000..050b6734f9 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveRemainingSpace.js @@ -0,0 +1,32 @@ +import styles from 'frontend/Foreground.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +const positions = { + above: 'spaceAbove', + below: 'spaceBelow' +}; + +export function toHaveRemainingSpace(subject, options = {}) { + const foreground = getElement(subject).querySelector(`.${styles.Foreground}`); + + const mismatches = Object.keys(positions).reduce((result, position) => { + if (position in options) { + const actual = foreground.classList.contains(styles[positions[position]]); + + if (actual !== options[position]) { + result.push(`${position} ${JSON.stringify(options[position])} (found ${JSON.stringify(actual)})`); + } + } + + return result; + }, []); + + return { + pass: mismatches.length === 0, + message: () => + mismatches.length + ? `expected remaining vertical space to have ${mismatches.join(', ')}` + : 'expected section not to have the given remaining vertical space' + }; +} diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveSuppressedPadding.js b/entry_types/scrolled/package/spec/support/matchers/toHaveSuppressedPadding.js new file mode 100644 index 0000000000..1a560bd305 --- /dev/null +++ b/entry_types/scrolled/package/spec/support/matchers/toHaveSuppressedPadding.js @@ -0,0 +1,32 @@ +import styles from 'frontend/Foreground.module.css'; + +import {getElement} from 'testHelpers/matchers/getElement'; + +const sides = { + top: 'suppressedPaddingTop', + bottom: 'suppressedPaddingBottom' +}; + +export function toHaveSuppressedPadding(subject, options = {}) { + const foreground = getElement(subject).querySelector(`.${styles.Foreground}`); + + const mismatches = Object.keys(sides).reduce((result, side) => { + if (side in options) { + const actual = foreground.classList.contains(styles[sides[side]]); + + if (actual !== options[side]) { + result.push(`${side} ${JSON.stringify(options[side])} (found ${JSON.stringify(actual)})`); + } + } + + return result; + }, []); + + return { + pass: mismatches.length === 0, + message: () => + mismatches.length + ? `expected section padding suppression to have ${mismatches.join(', ')}` + : 'expected section padding suppression not to match' + }; +} diff --git a/entry_types/scrolled/package/spec/support/pageObjects/index.js b/entry_types/scrolled/package/spec/support/pageObjects/index.js index 95951c9688..ec51bc4c30 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/index.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/index.js @@ -2,11 +2,6 @@ import React from 'react'; import {renderInEntry} from '..'; import {Entry} from 'frontend/Entry'; -import foregroundStyles from 'frontend/Foreground.module.css'; -import sharedTransitionStyles from 'frontend/transitions/shared.module.css'; -import centerLayoutStyles from 'frontend/layouts/Center.module.css'; -import twoColumnLayoutStyles from 'frontend/layouts/TwoColumn.module.css'; -import boxBoundaryMarginStyles from 'frontend/foregroundBoxes/BoxBoundaryMargin.module.css'; import {StaticPreview} from 'frontend/useScrollPositionLifecycle'; import {api} from 'frontend/api'; @@ -146,7 +141,6 @@ function fakeContentElementBoundingClientRectsByTestId(container, rectsByTestId) function createSectionPageObject(el) { const selectionRect = el.closest('[aria-label="Select section"]'); - const foreground = el.querySelector(`.${foregroundStyles.Foreground}`); return { el, @@ -174,34 +168,6 @@ function createSectionPageObject(el) { fireEvent.mouseDown(getByTitle('Edit section transition after')); }, - hasSuppressedTopPadding() { - return foreground.classList.contains(foregroundStyles.suppressedPaddingTop); - }, - - hasSuppressedBottomPadding() { - return foreground.classList.contains(foregroundStyles.suppressedPaddingBottom); - }, - - hasForcedPadding() { - return foreground.classList.contains(foregroundStyles.forcePadding); - }, - - hasRemainingSpaceAbove() { - return foreground.classList.contains(foregroundStyles.spaceAbove); - }, - - hasRemainingSpaceBelow() { - return foreground.classList.contains(foregroundStyles.spaceBelow); - }, - - hasFadedOutForeground() { - return foreground.classList.contains(sharedTransitionStyles.fadedOut); - }, - - usesPerElementFadeTransition() { - return foreground.classList.contains(sharedTransitionStyles.perElementFade); - }, - getPaddingIndicator(position) { const {getByLabelText} = within(selectionRect); const labels = { @@ -214,16 +180,6 @@ function createSectionPageObject(el) { selectPadding(position) { fireEvent.mouseDown(selectionRect); fireEvent.click(this.getPaddingIndicator(position)); - }, - - hasFirstBoxSuppressedTopMargin() { - const firstBox = foreground.querySelector(`.${boxBoundaryMarginStyles.noTopMargin}`); - return !!firstBox; - }, - - hasConstrainedContentWidth() { - return !!(el.querySelector(`.${twoColumnLayoutStyles.constrainContentWidth}`) || - el.querySelector(`.${centerLayoutStyles.constrainContentWidth}`)); } } } From 1f30ee0029814c79b40d359bf0e6d3813dff92f5 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 21:07:54 +0200 Subject: [PATCH 08/11] Group test matchers into per-subject subdirectories --- .../internal/testing_conventions/matchers.md | 6 +++--- .../{ => contentElement}/toHaveAlignment.js | 0 .../toHaveContentElementMargin.js | 0 .../{ => contentElement}/toHaveScrollSpace.js | 0 .../package/spec/support/matchers/index.js | 20 +++++++++---------- .../toHaveConstrainedContentWidth.js | 0 .../{ => section}/toHaveFadedOutForeground.js | 0 .../toHaveFirstBoxSuppressedTopMargin.js | 0 .../{ => section}/toHaveForcedPadding.js | 0 .../toHavePerElementFadeTransition.js | 0 .../{ => section}/toHaveRemainingSpace.js | 0 .../{ => section}/toHaveSuppressedPadding.js | 0 12 files changed, 13 insertions(+), 13 deletions(-) rename entry_types/scrolled/package/spec/support/matchers/{ => contentElement}/toHaveAlignment.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => contentElement}/toHaveContentElementMargin.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => contentElement}/toHaveScrollSpace.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHaveConstrainedContentWidth.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHaveFadedOutForeground.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHaveFirstBoxSuppressedTopMargin.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHaveForcedPadding.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHavePerElementFadeTransition.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHaveRemainingSpace.js (100%) rename entry_types/scrolled/package/spec/support/matchers/{ => section}/toHaveSuppressedPadding.js (100%) diff --git a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md index 2651da42c5..b8cc9e35eb 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md @@ -38,8 +38,8 @@ Plugin authors need these to test their own components. | `toContainContentElementBox({boxShadow, borderRadius, outlineColor})` | A `` is present (bare call), and the requested theme styles are applied. `.not` asserts absence. | | `toContainFitViewport({aspectRatio})` | A `` is present (bare call) with the requested aspect ratio. `.not` asserts absence. | -**Internal matchers** (`spec/support/matchers/`, enabled with -`useContentElementLayoutMatchers()`) cover framework state applied +**Internal matchers** (`spec/support/matchers/contentElement/`, enabled +with `useContentElementLayoutMatchers()`) cover framework state applied *around* the element. They depend on the entry-level layout, so the subject must come from `renderEntry` — `renderInContentElement` does not render these wrappers. @@ -50,7 +50,7 @@ render these wrappers. | `toHaveScrollSpace()` | The element sits inside a scroll-space wrapper. | | `toHaveAlignment(value)` | The element carries the alignment class for the given value. | -**Section matchers** (`spec/support/matchers/`, enabled with +**Section matchers** (`spec/support/matchers/section/`, enabled with `useSectionMatchers()`) assert a section's foreground/layout state. The subject is a section page object (`getSectionByPermaId(...)`), so they also require `renderEntry`. diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveAlignment.js b/entry_types/scrolled/package/spec/support/matchers/contentElement/toHaveAlignment.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveAlignment.js rename to entry_types/scrolled/package/spec/support/matchers/contentElement/toHaveAlignment.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveContentElementMargin.js b/entry_types/scrolled/package/spec/support/matchers/contentElement/toHaveContentElementMargin.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveContentElementMargin.js rename to entry_types/scrolled/package/spec/support/matchers/contentElement/toHaveContentElementMargin.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveScrollSpace.js b/entry_types/scrolled/package/spec/support/matchers/contentElement/toHaveScrollSpace.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveScrollSpace.js rename to entry_types/scrolled/package/spec/support/matchers/contentElement/toHaveScrollSpace.js diff --git a/entry_types/scrolled/package/spec/support/matchers/index.js b/entry_types/scrolled/package/spec/support/matchers/index.js index 83babf6f60..59471041ac 100644 --- a/entry_types/scrolled/package/spec/support/matchers/index.js +++ b/entry_types/scrolled/package/spec/support/matchers/index.js @@ -1,14 +1,14 @@ -import {toHaveContentElementMargin} from './toHaveContentElementMargin'; -import {toHaveScrollSpace} from './toHaveScrollSpace'; -import {toHaveAlignment} from './toHaveAlignment'; +import {toHaveContentElementMargin} from './contentElement/toHaveContentElementMargin'; +import {toHaveScrollSpace} from './contentElement/toHaveScrollSpace'; +import {toHaveAlignment} from './contentElement/toHaveAlignment'; -import {toHaveSuppressedPadding} from './toHaveSuppressedPadding'; -import {toHaveRemainingSpace} from './toHaveRemainingSpace'; -import {toHaveForcedPadding} from './toHaveForcedPadding'; -import {toHaveFadedOutForeground} from './toHaveFadedOutForeground'; -import {toHavePerElementFadeTransition} from './toHavePerElementFadeTransition'; -import {toHaveFirstBoxSuppressedTopMargin} from './toHaveFirstBoxSuppressedTopMargin'; -import {toHaveConstrainedContentWidth} from './toHaveConstrainedContentWidth'; +import {toHaveSuppressedPadding} from './section/toHaveSuppressedPadding'; +import {toHaveRemainingSpace} from './section/toHaveRemainingSpace'; +import {toHaveForcedPadding} from './section/toHaveForcedPadding'; +import {toHaveFadedOutForeground} from './section/toHaveFadedOutForeground'; +import {toHavePerElementFadeTransition} from './section/toHavePerElementFadeTransition'; +import {toHaveFirstBoxSuppressedTopMargin} from './section/toHaveFirstBoxSuppressedTopMargin'; +import {toHaveConstrainedContentWidth} from './section/toHaveConstrainedContentWidth'; export function useContentElementLayoutMatchers() { beforeEach(() => { diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveConstrainedContentWidth.js b/entry_types/scrolled/package/spec/support/matchers/section/toHaveConstrainedContentWidth.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveConstrainedContentWidth.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHaveConstrainedContentWidth.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveFadedOutForeground.js b/entry_types/scrolled/package/spec/support/matchers/section/toHaveFadedOutForeground.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveFadedOutForeground.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHaveFadedOutForeground.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveFirstBoxSuppressedTopMargin.js b/entry_types/scrolled/package/spec/support/matchers/section/toHaveFirstBoxSuppressedTopMargin.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveFirstBoxSuppressedTopMargin.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHaveFirstBoxSuppressedTopMargin.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveForcedPadding.js b/entry_types/scrolled/package/spec/support/matchers/section/toHaveForcedPadding.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveForcedPadding.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHaveForcedPadding.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHavePerElementFadeTransition.js b/entry_types/scrolled/package/spec/support/matchers/section/toHavePerElementFadeTransition.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHavePerElementFadeTransition.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHavePerElementFadeTransition.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveRemainingSpace.js b/entry_types/scrolled/package/spec/support/matchers/section/toHaveRemainingSpace.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveRemainingSpace.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHaveRemainingSpace.js diff --git a/entry_types/scrolled/package/spec/support/matchers/toHaveSuppressedPadding.js b/entry_types/scrolled/package/spec/support/matchers/section/toHaveSuppressedPadding.js similarity index 100% rename from entry_types/scrolled/package/spec/support/matchers/toHaveSuppressedPadding.js rename to entry_types/scrolled/package/spec/support/matchers/section/toHaveSuppressedPadding.js From ba4759ecc91550d8f5c97e9446a770117652531a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 21:53:12 +0200 Subject: [PATCH 09/11] Move inline editing actions to inline editing page objects --- .../package/spec/support/pageObjects/index.js | 86 ++---------------- .../spec/support/pageObjects/inlineEditing.js | 89 ++++++++++++++++++- 2 files changed, 96 insertions(+), 79 deletions(-) diff --git a/entry_types/scrolled/package/spec/support/pageObjects/index.js b/entry_types/scrolled/package/spec/support/pageObjects/index.js index ec51bc4c30..9691b68e69 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/index.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/index.js @@ -5,14 +5,15 @@ import {Entry} from 'frontend/Entry'; import {StaticPreview} from 'frontend/useScrollPositionLifecycle'; import {api} from 'frontend/api'; -import {act, fireEvent, queryHelpers, queries, within} from '@testing-library/react' +import {act, queryHelpers, queries} from '@testing-library/react' import {simulateScrollingIntoView} from '../fakeIntersectionObserver'; export function renderEntry({ seed, consent, isStaticPreview, phonePlatform, entryProps, contentElement, - contentElementFactory = createContentElementPageObject + contentElementFactory = createContentElementPageObject, + sectionFactory = createSectionPageObject } = {}) { const effectiveSeed = contentElement ? mergeContentElement(seed, contentElement) : seed; const entry = ; @@ -22,7 +23,7 @@ export function renderEntry({ consent, phonePlatform, wrapper: isStaticPreview ? StaticPreview : null, - queries: {...queries, ...buildPageObjectQueries({contentElementFactory})} + queries: {...queries, ...buildPageObjectQueries({contentElementFactory, sectionFactory})} }); return { @@ -67,7 +68,7 @@ export function usePageObjects() { }); } -function buildPageObjectQueries({contentElementFactory}) { +function buildPageObjectQueries({contentElementFactory, sectionFactory}) { return { getSectionByPermaId(container, permaId) { const el = queryHelpers.queryByAttribute('id', @@ -81,7 +82,7 @@ function buildPageObjectQueries({contentElementFactory}) { ); } - return createSectionPageObject(el); + return sectionFactory(el); }, getContentElementByTestId(container, testId) { @@ -139,89 +140,18 @@ function fakeContentElementBoundingClientRectsByTestId(container, rectsByTestId) }); } -function createSectionPageObject(el) { - const selectionRect = el.closest('[aria-label="Select section"]'); - +export function createSectionPageObject(el) { return { el, simulateScrollingIntoView() { act(() => simulateScrollingIntoView(el)); - }, - - select() { - fireEvent.mouseDown(selectionRect); - }, - - clickAddContentElement() { - const {getByTitle} = within(selectionRect); - fireEvent.click(getByTitle('Add content element')); - }, - - clickEditTransitionBefore() { - const {getByTitle} = within(selectionRect); - fireEvent.mouseDown(getByTitle('Edit section transition before')); - }, - - clickEditTransitionAfter() { - const {getByTitle} = within(selectionRect); - fireEvent.mouseDown(getByTitle('Edit section transition after')); - }, - - getPaddingIndicator(position) { - const {getByLabelText} = within(selectionRect); - const labels = { - top: 'Edit top padding', - bottom: 'Edit bottom padding' - }; - return getByLabelText(labels[position]); - }, - - selectPadding(position) { - fireEvent.mouseDown(selectionRect); - fireEvent.click(this.getPaddingIndicator(position)); } } } export function createContentElementPageObject(el) { - const selectionRect = el.closest('[aria-label="Select content element"]'); - return { - el, - - getMarginIndicator(position) { - const {getByLabelText} = within(selectionRect); - const labels = { - top: 'Top margin', - bottom: 'Bottom margin' - }; - return getByLabelText(labels[position]); - }, - - select() { - fireEvent.click(selectionRect); - }, - - isSelected() { - return selectionRect.getAttribute('aria-selected') === 'true'; - }, - - clickInsertAfterButton() { - const {getByTitle} = within(selectionRect); - fireEvent.click(getByTitle('Insert content element after')); - }, - - drag(at, otherContentElement) { - const {getByTitle} = within(selectionRect); - fireEvent.dragStart(getByTitle('Drag to move')); - otherContentElement._drop(at); - }, - - _drop(at) { - const {getByTestId} = within(selectionRect); - const target = getByTestId(`drop-${at}`); - fireEvent.drop(target); - } + el } } diff --git a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js index affe89b27d..4a0ef5f91f 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js @@ -1,4 +1,4 @@ -import {act, fireEvent} from '@testing-library/react'; +import {act, fireEvent, within} from '@testing-library/react'; import {useFakeTranslations} from 'pageflow/testHelpers'; import {loadInlineEditingExtensions} from 'frontend/inlineEditing'; @@ -8,11 +8,15 @@ import badgeStyles from 'review/Badge.module.css'; import {useFakeParentWindow} from '../fakeWindows'; import { renderEntry as baseRenderEntry, + createSectionPageObject as baseCreateSectionPageObject, + createContentElementPageObject as baseCreateContentElementPageObject, usePageObjects } from './index'; export function renderEntry({commenting, ...options} = {}) { const result = baseRenderEntry({ + sectionFactory: createInlineEditingSectionPageObject, + contentElementFactory: createInlineEditingContentElementPageObject, ...options, entryProps: {commentingInitialState: commenting} }); @@ -24,6 +28,89 @@ export function renderEntry({commenting, ...options} = {}) { }; } +function createInlineEditingSectionPageObject(el) { + const selectionRect = el.closest('[aria-label="Select section"]'); + + return { + ...baseCreateSectionPageObject(el), + + select() { + fireEvent.mouseDown(selectionRect); + }, + + clickAddContentElement() { + const {getByTitle} = within(selectionRect); + fireEvent.click(getByTitle('Add content element')); + }, + + clickEditTransitionBefore() { + const {getByTitle} = within(selectionRect); + fireEvent.mouseDown(getByTitle('Edit section transition before')); + }, + + clickEditTransitionAfter() { + const {getByTitle} = within(selectionRect); + fireEvent.mouseDown(getByTitle('Edit section transition after')); + }, + + getPaddingIndicator(position) { + const {getByLabelText} = within(selectionRect); + const labels = { + top: 'Edit top padding', + bottom: 'Edit bottom padding' + }; + return getByLabelText(labels[position]); + }, + + selectPadding(position) { + fireEvent.mouseDown(selectionRect); + fireEvent.click(this.getPaddingIndicator(position)); + } + }; +} + +function createInlineEditingContentElementPageObject(el) { + const selectionRect = el.closest('[aria-label="Select content element"]'); + + return { + ...baseCreateContentElementPageObject(el), + + getMarginIndicator(position) { + const {getByLabelText} = within(selectionRect); + const labels = { + top: 'Top margin', + bottom: 'Bottom margin' + }; + return getByLabelText(labels[position]); + }, + + select() { + fireEvent.click(selectionRect); + }, + + isSelected() { + return selectionRect.getAttribute('aria-selected') === 'true'; + }, + + clickInsertAfterButton() { + const {getByTitle} = within(selectionRect); + fireEvent.click(getByTitle('Insert content element after')); + }, + + drag(at, otherContentElement) { + const {getByTitle} = within(selectionRect); + fireEvent.dragStart(getByTitle('Drag to move')); + otherContentElement._drop(at); + }, + + _drop(at) { + const {getByTestId} = within(selectionRect); + const target = getByTestId(`drop-${at}`); + fireEvent.drop(target); + } + }; +} + export function useInlineEditingPageObjects() { useFakeParentWindow(); From b0bc6a047a7233d37447fdd1e86d58747c5fb8a5 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 10 Jun 2026 22:04:15 +0200 Subject: [PATCH 10/11] Restructure testing conventions guides Reorganize the testing conventions guides so they work as a reference for coding agents and collaborators extending the test suite: each guide now leads with a decision procedure, followed by rationale and a scannable checklist that frontloads review judgment. Volatile detail such as matcher signatures and import paths defers to the source files to keep the guides from drifting out of date; examples remain only where they disambiguate a rule. --- .../doc/internal/testing_conventions.md | 3 +- .../internal/testing_conventions/matchers.md | 86 ++++----- .../testing_conventions/render-helpers.md | 63 ++++--- .../testing_conventions/spec-layout.md | 176 ++++++++++-------- 4 files changed, 180 insertions(+), 148 deletions(-) diff --git a/entry_types/scrolled/doc/internal/testing_conventions.md b/entry_types/scrolled/doc/internal/testing_conventions.md index 60f5978d26..e97550bc01 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions.md +++ b/entry_types/scrolled/doc/internal/testing_conventions.md @@ -26,8 +26,7 @@ each area in depth — read the one matching your task. ## Guides - [Render helpers](testing_conventions/render-helpers.md) — the two kinds - of helper, how to pick one, custom wrappers, and the `useXxx` setup - hooks. + 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. diff --git a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md index b8cc9e35eb..fd7730544f 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/matchers.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/matchers.md @@ -26,53 +26,53 @@ expect(getContentElementByTestId('probe')).toHaveAlignment('right'); ## Public vs internal The dividing line is **what the content element controls** vs **what the -framework applies around it**. - -**Public matchers** (`src/testHelpers/matchers/`, shipped via -`pageflow-scrolled/testHelpers`, enabled with `useContentElementMatchers()`) -cover chrome a content element opts into through framework components. -Plugin authors need these to test their own components. - -| Matcher | Asserts | -| --- | --- | -| `toContainContentElementBox({boxShadow, borderRadius, outlineColor})` | A `` is present (bare call), and the requested theme styles are applied. `.not` asserts absence. | -| `toContainFitViewport({aspectRatio})` | A `` is present (bare call) with the requested aspect ratio. `.not` asserts absence. | - -**Internal matchers** (`spec/support/matchers/contentElement/`, enabled -with `useContentElementLayoutMatchers()`) cover framework state applied -*around* the element. They depend on the entry-level layout, so the -subject must come from `renderEntry` — `renderInContentElement` does not -render these wrappers. - -| Matcher | Asserts | -| --- | --- | -| `toHaveContentElementMargin({top, bottom, prevBottom, topTrimmed})` | A content element margin wrapper is present; the `--margin-*` custom properties (raw CSS strings) match; `topTrimmed` checks the trim class. `.not` asserts absence. | -| `toHaveScrollSpace()` | The element sits inside a scroll-space wrapper. | -| `toHaveAlignment(value)` | The element carries the alignment class for the given value. | - -**Section matchers** (`spec/support/matchers/section/`, enabled with -`useSectionMatchers()`) assert a section's foreground/layout state. 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`. - -| Matcher | Asserts | -| --- | --- | -| `toHaveSuppressedPadding({top, bottom})` | The foreground suppresses top/bottom padding. | -| `toHaveRemainingSpace({above, below})` | The foreground keeps remaining vertical space above/below the content. | -| `toHaveForcedPadding()` | The foreground forces padding (inline editing). | -| `toHaveFadedOutForeground()` | The foreground is faded out by a transition. | -| `toHavePerElementFadeTransition()` | The section uses the per-element fade transition. | -| `toHaveFirstBoxSuppressedTopMargin()` | The first foreground box has its top margin suppressed. | -| `toHaveConstrainedContentWidth()` | The section constrains its content width. | - -These sets are registered through the `useXxx` hooks listed under +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 -public matchers through `useContentElementMatchers` and internal ones -through `useContentElementLayoutMatchers` so specs opt in explicitly, -mirroring `usePageObjects`. +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. diff --git a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md index 2fa1d5796c..437acc5368 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md @@ -6,7 +6,7 @@ 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. They +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. @@ -14,20 +14,26 @@ content element plugins can use them too. official `Entry` component and interact with it via *page objects*. They live in `spec/support/` and are internal to this package. -| Helper | Import from | Scope | -| --- | --- | --- | -| `renderInEntry` | `pageflow-scrolled/testHelpers` | Entry state + `RootProviders`. For any component that needs entry state but nothing more. | -| `renderHookInEntry` | `pageflow-scrolled/testHelpers` | Same as `renderInEntry`, for selector hooks that read entry state. | -| `renderInContentElement` | `pageflow-scrolled/testHelpers` | Content-element scope: attributes, lifecycle, command emitter, optional inline-editing context. For components rendered *inside* a content element. | -| `renderWithReviewState` | `pageflow-scrolled/testHelpers` | `ReviewStateProvider` only. For code in `src/review/`. | -| `renderEntry` | `support/pageObjects` | Full `Entry`. Returns page-object queries (`getContentElementByTestId`, `getSectionByPermaId`). For cross-cutting frontend features and integration behavior. | +The import path follows from the kind: 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 a further scoped fixture for `src/review/` +code — package-internal (imported from `testHelpers/renderWithReviewState`, +not the public barrel), setting up `ReviewStateProvider` only. ## Picking a helper -Pick the smallest-scope helper sufficient for the behavior under test. -The path of the spec file is the strongest signal: `spec/-spec.js` -mirrors `src/.js`, and the scope at that path determines which -helper to use. +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//.js`) → `renderInContentElement`, asserting with [matchers](matchers.md). @@ -39,22 +45,11 @@ helper to use. layout) → `renderEntry` with page objects. `renderInContentElement` returns extra controls beyond the -`@testing-library/react` result: `simulateScrollPosition`, -`triggerEditorCommand`, and `simulateStorylineMode`. Its `inlineEditing` -option opts the element into an inline-editing context — pass `true` for -editable defaults or an object (`{isEditable, isSelected, -transientState}`) to override. - -## Custom wrappers - -`renderInEntry` and `renderInContentElement` accept a `wrapper` option: a -`({children}) => JSX` component that adds extra providers around the -rendered tree. Use it for a one-off provider combination a test needs. - -When a combination is common enough to be discoverable, add a named -option to the render helper that installs the providers internally — -this is how `renderInContentElement`'s `inlineEditing` option works — -rather than repeating the same custom `wrapper` across specs. +`@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 @@ -73,3 +68,15 @@ in the order they appear below. (`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. diff --git a/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md b/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md index 2ff058df23..915b6db7d5 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/spec-layout.md @@ -3,81 +3,107 @@ Part of the [Testing Conventions](../testing_conventions.md) guide, which defines the **kind / scope / context / unit** vocabulary used here. -Specs mirror source structure. Two rules govern when a spec becomes a -directory: - -1. A **`src/Foo.js`** source file corresponds to a **`spec/Foo-spec.js`** - spec file. When it grows large enough to split into topics, it becomes - a directory: topic files live at `spec/Foo/features/-spec.js`. -2. A **`src/Foo/`** source directory corresponds to a **`spec/Foo/`** spec - directory, with one `spec/Foo/-spec.js` per - `src/Foo/.js`. Topic splits of the main component live in - `spec/Foo/features/` alongside the helper specs. - -The `features/` sub-namespace is the *only* place topic splits live, -regardless of whether the unit under test is a `.js` file or a `/` -directory. This keeps spec layout invariant under -`src/Foo.js → src/Foo/index.js` refactorings. - -It follows that a unit's behavior tests live in exactly one place: the -flat `spec/Foo-spec.js`, or — once split — `spec/Foo/features/`, but -**never both**. A `spec/Foo-spec.js` next to a `spec/Foo/features/` -directory means a `src/Foo.js → src/Foo/index.js` refactoring left the -top-level spec behind; fold it into `features/`. (Helper and -sub-component unit specs at `spec/Foo/-spec.js` test *different* -units and may sit beside a flat `spec/Foo-spec.js` — it is specifically -`Foo-spec.js` together with `Foo/features/` that must not coexist.) - -A *unit spec* mirrors a single source unit and stays at the directory -level, named after that unit. A unit is usually a separate file -(`spec/Foo/-spec.js` for `src/Foo/.js`), but can also be -a **named export** of the main file — e.g. `AudioPlayer/index.js` -exports both the `AudioPlayer` component and a `processSources` helper, -so `processSources-spec.js` mirrors that export and stays at the -directory level. The component's own rendered behavior (e.g. its -structured-data output) is a topic split, so it lives under `features/` -(`features/structuredData-spec.js`). - -`features/` holds the remaining *behavior-grouped* tests, written -against the unit's stable public interface; these tend to outlive the -internal helpers they exercise. The path's scope and context determine -which render helper and setup hooks the specs in that directory use. - -**`features/` attaches to the level of the unit under test.** The -directory path identifies the unit — whatever has a source counterpart -at that path — and `features/` nests *under* it to group that unit's -behavior topics; it never floats up to the context root. So the -`EditableText` component (`src/frontend/inlineEditing/EditableText/`) -owns `spec/frontend/inlineEditing/EditableText/features/`, with its -helper specs (`blocks-spec.js`, `marks-spec.js`, …) alongside. -Conversely, behavior with no single source counterpart in a context — -the inline-editing integration itself (`contentElementSelection`, -`marginIndicator`) — lives directly in that context's -`spec/frontend/inlineEditing/features/`. The render 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. +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/-spec.js` | +| `src/Foo/.js` | `spec/Foo/-spec.js` | +| a **named export** of `src/Foo/index.js` | `spec/Foo/-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/.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: specs under -`spec/frontend/inlineEditing/` load the inline-editing extension, those -under `spec/frontend/commenting/` the commenting extension, and those -directly under `spec/frontend/` neither. When a unit behaves differently -across these contexts, it gets one spec per context, named after the -unit, in the matching directory — the path conveys the context, so the -filenames stay identical. - -Prefer committing to the public interface over its wiring. When a unit's -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`, rather than white-box-driving -the replacement mechanism (which couples the spec to how providers are -registered, not to the contract). For example `usePhonePlatform` is -asserted via a probe content element in -`spec/frontend/features/usePhonePlatform-spec.js` (default provider, -reads the `browser`) and -`spec/frontend/inlineEditing/features/usePhonePlatform-spec.js` (the -inline-editing provider, driven by the editor's emulation-mode -messages). Both live in `features/` because they exercise rendered -behavior; a spec that instead drove the hook directly with -`renderHookInEntry` would be a dir-level white-box unit spec. +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/-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. From 093caab07be873438e7da72f787645db716d98cf Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 11 Jun 2026 11:49:02 +0200 Subject: [PATCH 11/11] Move renderWithReviewState to spec/support `renderWithReviewState` only sets up state for testing review UI components, which no plugin is expected to extend, so it does not belong in the public `testHelpers` package. Moving it alongside the other package-internal helpers keeps the shipped `testHelpers` surface uniformly public and drops test-only code from the build. --- .../doc/internal/testing_conventions/render-helpers.md | 8 ++++---- .../scrolled/package/spec/review/ThreadList-spec.js | 2 +- .../scrolled/package/spec/review/ThreadsBadge-spec.js | 2 +- .../testHelpers => spec/support}/renderWithReviewState.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) rename entry_types/scrolled/package/{src/testHelpers => spec/support}/renderWithReviewState.js (81%) diff --git a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md index 437acc5368..ad7cba5a02 100644 --- a/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md +++ b/entry_types/scrolled/doc/internal/testing_conventions/render-helpers.md @@ -14,7 +14,7 @@ content element plugins can use them too. 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: scoped fixtures from +The import path follows from the kind: public scoped fixtures from `pageflow-scrolled/testHelpers`; page objects and `renderEntry` from `support/pageObjects`. @@ -25,9 +25,9 @@ The import path follows from the kind: scoped fixtures from | `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 a further scoped fixture for `src/review/` -code — package-internal (imported from `testHelpers/renderWithReviewState`, -not the public barrel), setting up `ReviewStateProvider` only. +`renderWithReviewState` is an internal scoped fixture for `src/review/` +UI, imported from `support/renderWithReviewState`; it sets up +`ReviewStateProvider` only. ## Picking a helper diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 7356dfaf08..2576edf6bf 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -4,7 +4,7 @@ import userEvent from '@testing-library/user-event'; import {useFakeTranslations} from 'pageflow/testHelpers'; import {ThreadList} from 'review/ThreadList'; -import {renderWithReviewState} from 'testHelpers/renderWithReviewState'; +import {renderWithReviewState} from 'support/renderWithReviewState'; describe('ThreadList', () => { useFakeTranslations({ diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index e19b0c8f04..908d918099 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -2,7 +2,7 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; import {ThreadsBadge} from 'review/ThreadsBadge'; -import {renderWithReviewState} from 'testHelpers/renderWithReviewState'; +import {renderWithReviewState} from 'support/renderWithReviewState'; import {fakeParentWindow} from 'support'; describe('ThreadsBadge', () => { diff --git a/entry_types/scrolled/package/src/testHelpers/renderWithReviewState.js b/entry_types/scrolled/package/spec/support/renderWithReviewState.js similarity index 81% rename from entry_types/scrolled/package/src/testHelpers/renderWithReviewState.js rename to entry_types/scrolled/package/spec/support/renderWithReviewState.js index b0039bcf75..39c2ef7236 100644 --- a/entry_types/scrolled/package/src/testHelpers/renderWithReviewState.js +++ b/entry_types/scrolled/package/spec/support/renderWithReviewState.js @@ -1,7 +1,7 @@ import React from 'react'; import {render} from '@testing-library/react'; -import {ReviewStateProvider} from '../review/ReviewStateProvider'; +import {ReviewStateProvider} from 'review/ReviewStateProvider'; export function renderWithReviewState(ui, {commentThreads = [], currentUser = null} = {}) { return render(