From e77e174c929e9830e375113192429aba9d02c9c7 Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Wed, 16 Sep 2026 07:20:40 -0700 Subject: [PATCH] Refactor post image rendering and cover image helpers Extract the pure failed-image set transition into a testable service, DRY the acceptance no-link/no-image assertions, and add property tests for image URL detection, alt text, and image rendering. By refactorer. --- psf-memo-client/acceptance/lib/handlers.js | 21 ++-- .../src/components/post-feed/post-content.js | 8 +- psf-memo-client/src/services/failed-images.js | 21 ++++ .../property/post-content.property.test.js | 111 ++++++++++++++++++ .../test/property/post-links.property.test.js | 107 ++++++++++++++++- .../test/unit/failed-images.test.js | 37 ++++++ 6 files changed, 288 insertions(+), 17 deletions(-) create mode 100644 psf-memo-client/src/services/failed-images.js create mode 100644 psf-memo-client/test/property/post-content.property.test.js create mode 100644 psf-memo-client/test/unit/failed-images.test.js diff --git a/psf-memo-client/acceptance/lib/handlers.js b/psf-memo-client/acceptance/lib/handlers.js index 4c7ec00..99396eb 100644 --- a/psf-memo-client/acceptance/lib/handlers.js +++ b/psf-memo-client/acceptance/lib/handlers.js @@ -3173,11 +3173,7 @@ const handlers = [ name: 'feed shows no link', pattern: /^the feed shows no link$/, run (m, example, world) { - const rendered = getRenderedFeed(world) - const found = rendered.some((html) => anchorsIn(html).length > 0) - if (found) { - throw new Error('Feed unexpectedly shows a link.') - } + assertFeedHasNoElement(world, anchorsIn, 'Feed unexpectedly shows a link.') } }, { @@ -3237,11 +3233,7 @@ const handlers = [ name: 'feed shows no image', pattern: /^the feed shows no image$/, run (m, example, world) { - const rendered = getRenderedFeed(world) - const found = rendered.some((html) => imagesIn(html).length > 0) - if (found) { - throw new Error('Feed unexpectedly shows an image.') - } + assertFeedHasNoElement(world, imagesIn, 'Feed unexpectedly shows an image.') } }, { @@ -3279,6 +3271,15 @@ function getRenderedFeed (world) { return world.renderedFeed } +// Fail when any rendered feed HTML contains an element matched by extract. +function assertFeedHasNoElement (world, extract, message) { + const rendered = getRenderedFeed(world) + const found = rendered.some((html) => extract(html).length > 0) + if (found) { + throw new Error(message) + } +} + // Extract the anchors from a rendered HTML string. The acceptance adapter // renders a small, controlled HTML subset, so a regex match is sufficient. function anchorsIn (html) { diff --git a/psf-memo-client/src/components/post-feed/post-content.js b/psf-memo-client/src/components/post-feed/post-content.js index 1c61195..df4dc8e 100644 --- a/psf-memo-client/src/components/post-feed/post-content.js +++ b/psf-memo-client/src/components/post-feed/post-content.js @@ -16,6 +16,7 @@ const { isImageUrl, imageAltText } = require('../../services/post-links') +const { addFailedImage } = require('../../services/failed-images') // A post image that falls back to a plain link if the image fails to load. function PostImage ({ href, alt, failed, onError }) { @@ -56,12 +57,7 @@ function PostContent ({ text = '', initialFailedImages }) { const children = [] const failImage = (href) => { - setFailedImages((previous) => { - if (previous.has(href)) return previous - const next = new Set(previous) - next.add(href) - return next - }) + setFailedImages((previous) => addFailedImage(previous, href)) } for (const segment of parsePostText(text)) { diff --git a/psf-memo-client/src/services/failed-images.js b/psf-memo-client/src/services/failed-images.js new file mode 100644 index 0000000..921d33f --- /dev/null +++ b/psf-memo-client/src/services/failed-images.js @@ -0,0 +1,21 @@ +/* + Pure state transition for tracking post images that failed to load. + + This is kept free of React and the DOM so the transition can be unit tested + directly. The PostContent component uses it as the reducer for its + failed-image state. +*/ + +/** + * Add a failed image URL to the set. Returns the same set when the URL is + * already present, so React can bail out of a no-op update; otherwise returns + * a new set containing the added URL. The input set is never mutated. + */ +function addFailedImage (failedImages, href) { + if (failedImages.has(href)) return failedImages + const next = new Set(failedImages) + next.add(href) + return next +} + +module.exports = { addFailedImage } diff --git a/psf-memo-client/test/property/post-content.property.test.js b/psf-memo-client/test/property/post-content.property.test.js new file mode 100644 index 0000000..bd13c98 --- /dev/null +++ b/psf-memo-client/test/property/post-content.property.test.js @@ -0,0 +1,111 @@ +/* + Property tests for the post content renderer. + + The unit tests probe PostContent at a few fixed fixtures. These properties + pin down the rendering invariants for URL-like tokens over broad random + inputs: + + - An image URL renders as an inside a new-tab anchor, keeps its full + URL as the src (query string included), and is never repeated as visible + text. + - A non-image URL renders as a plain new-tab anchor with no . + - Surrounding text is preserved and rendering is deterministic. +*/ + +'use strict' + +const test = require('node:test') +const React = require('react') +const ReactDOMServer = require('react-dom/server') +const { seededRandom, forAll, intGen } = require('./harness') +const PostContent = require('../../src/components/post-feed/post-content') + +const rng = seededRandom(20260916) + +const IMAGE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'gif', 'webp', 'bmp'] + +function render (text) { + return ReactDOMServer.renderToStaticMarkup( + React.createElement(PostContent, { text }) + ) +} + +function randomName () { + const alphabet = 'abcdefghijklmnopqrstuvwxyz0123456789' + const n = intGen(rng, 1, 8)() + let out = '' + for (let i = 0; i < n; i++) out += alphabet[Math.floor(rng() * alphabet.length)] + return out +} + +function randomHost () { + return ['example.com', 'cdn.example.org', 'i.imgur.com'][Math.floor(rng() * 3)] +} + +// Tail values that are safe in HTML attributes (no &, ", <, >, or '). +function randomTail () { + const kind = Math.floor(rng() * 3) + if (kind === 0) return '' + if (kind === 1) return `?w=${intGen(rng, 1, 2000)()}` + return '#section' +} + +function randomImageUrl () { + const ext = IMAGE_EXTENSIONS[Math.floor(rng() * IMAGE_EXTENSIONS.length)] + const dir = ['', 'pics/', 'a/b/'][Math.floor(rng() * 3)] + return `https://${randomHost()}/${dir}${randomName()}.${ext}${randomTail()}` +} + +function randomNonImageUrl () { + if (Math.floor(rng() * 3) === 0) { + return `https://${randomHost()}/page${randomTail()}` + } + return `https://${randomHost()}/${randomName()}.svg${randomTail()}` +} + +function visibleText (html) { + return html.replace(/<[^>]+>/g, '') +} + +test('an image URL renders as an in a new-tab anchor and never as visible text', async () => { + await forAll( + () => randomImageUrl(), + async (url) => { + const html = render(`before ${url} after`) + if (!/]+src="/.test(html)) return false + if (!html.includes(`src="${url}"`)) return false + if (!/]+target="_blank"/.test(html)) return false + const text = visibleText(html) + if (text.includes(url)) return false + return text.includes('before') && text.includes('after') + }, + { label: 'post-content image rendering', samples: 300 } + ) +}) + +test('a non-image URL renders as a plain new-tab anchor with no image', async () => { + await forAll( + () => randomNonImageUrl(), + async (url) => { + const html = render(`before ${url} after`) + if (html.includes(']+target="_blank"/.test(html)) return false + return visibleText(html).includes('before') && visibleText(html).includes('after') + }, + { label: 'post-content non-image rendering', samples: 300 } + ) +}) + +test('rendering the same text is deterministic', async () => { + await forAll( + () => randomImageUrl(), + async (url) => { + const text = `x ${url} y` + const first = render(text) + const second = render(text) + return first === second + }, + { label: 'post-content determinism', samples: 200 } + ) +}) diff --git a/psf-memo-client/test/property/post-links.property.test.js b/psf-memo-client/test/property/post-links.property.test.js index 9600d45..16be945 100644 --- a/psf-memo-client/test/property/post-links.property.test.js +++ b/psf-memo-client/test/property/post-links.property.test.js @@ -22,7 +22,11 @@ const test = require('node:test') const assert = require('node:assert/strict') const { seededRandom, forAll, intGen } = require('./harness') -const { parsePostLinks } = require('../../src/services/post-links') +const { + parsePostLinks, + isImageUrl, + imageAltText +} = require('../../src/services/post-links') const rng = seededRandom(20260916) @@ -171,3 +175,104 @@ test('parsePostLinks stringifies nullish and non-string input', () => { assert.deepEqual(parsePostLinks(undefined), [{ type: 'text', text: '' }]) assert.deepEqual(parsePostLinks(12345), [{ type: 'text', text: '12345' }]) }) + +const IMAGE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'gif', 'webp', 'bmp'] +const NON_IMAGE_EXTENSIONS = ['svg', 'pdf', 'txt', 'html', 'json', 'js'] + +function randomCase (value) { + let out = '' + for (const ch of value) { + out += rng() < 0.5 ? ch.toLowerCase() : ch.toUpperCase() + } + return out +} + +function randomName () { + const alphabet = 'abcdefghijklmnopqrstuvwxyz0123456789' + const n = intGen(rng, 1, 10)() + let out = '' + for (let i = 0; i < n; i++) out += alphabet[Math.floor(rng() * alphabet.length)] + return out +} + +function randomHost () { + return ['example.com', 'cdn.example.org', 'i.imgur.com', 'images.example.net'][Math.floor(rng() * 4)] +} + +function randomQueryOrFragment () { + const kind = Math.floor(rng() * 4) + if (kind === 0) return '' + if (kind === 1) return `?w=${intGen(rng, 1, 2000)()}` + if (kind === 2) return '#section' + return '?a=1&b=2#frag' +} + +function randomImageUrl () { + const ext = randomCase(IMAGE_EXTENSIONS[Math.floor(rng() * IMAGE_EXTENSIONS.length)]) + const dir = ['', 'pics/', 'a/b/', 'img/'][Math.floor(rng() * 4)] + return `https://${randomHost()}/${dir}${randomName()}.${ext}${randomQueryOrFragment()}` +} + +function randomNonImageUrl () { + if (Math.floor(rng() * 3) === 0) { + return `https://${randomHost()}/page${randomQueryOrFragment()}` + } + const ext = NON_IMAGE_EXTENSIONS[Math.floor(rng() * NON_IMAGE_EXTENSIONS.length)] + return `https://${randomHost()}/${randomName()}.${ext}${randomQueryOrFragment()}` +} + +test('isImageUrl recognizes supported image extensions regardless of case or query', async () => { + await forAll( + () => randomImageUrl(), + async (url) => isImageUrl(url) === true, + { label: 'isImageUrl image urls', samples: 2000 } + ) +}) + +test('isImageUrl rejects non-image paths even when a query mentions an image', async () => { + await forAll( + () => randomNonImageUrl(), + async (url) => isImageUrl(url) === false, + { label: 'isImageUrl non-image urls', samples: 2000 } + ) +}) + +test('isImageUrl and imageAltText never throw and are deterministic for arbitrary input', async () => { + await forAll( + () => randomText(), + async (text) => { + let image, alt, imageAgain, altAgain + try { + image = isImageUrl(text) + alt = imageAltText(text) + imageAgain = isImageUrl(text) + altAgain = imageAltText(text) + } catch { + return false + } + if (typeof image !== 'boolean') return false + if (typeof alt !== 'string' || alt.length === 0) return false + return image === imageAgain && alt === altAgain + }, + { label: 'isImageUrl/imageAltText robustness', samples: 3000 } + ) +}) + +test('imageAltText returns the URL filename, ignoring query string and fragment', async () => { + await forAll( + () => randomImageUrl(), + async (url) => imageAltText(url) === new URL(url).pathname.split('/').pop(), + { label: 'imageAltText filename', samples: 2000 } + ) +}) + +test('parsePostLinks keeps an image URL intact so isImageUrl still recognizes it', async () => { + await forAll( + () => randomImageUrl(), + async (url) => { + const link = parsePostLinks(`see ${url} now`).find((segment) => segment.type === 'link') + return Boolean(link) && link.href === url && isImageUrl(link.href) === true + }, + { label: 'parsePostLinks image url round trip', samples: 2000 } + ) +}) diff --git a/psf-memo-client/test/unit/failed-images.test.js b/psf-memo-client/test/unit/failed-images.test.js new file mode 100644 index 0000000..7a0f1a8 --- /dev/null +++ b/psf-memo-client/test/unit/failed-images.test.js @@ -0,0 +1,37 @@ +/* + Unit tests for the failed-image state transition used by the post renderer. + + The transition is pure: it either returns the same set (when the URL is + already tracked, so React can skip a re-render) or a new set with the URL + added, and it never mutates the input set. +*/ + +'use strict' + +const test = require('node:test') +const assert = require('node:assert/strict') +const { addFailedImage } = require('../../src/services/failed-images') + +const URL_A = 'https://i.imgur.com/swCI56T.jpeg' +const URL_B = 'https://cdn.example.com/pics/Sunset.PNG' + +test('addFailedImage returns a new set containing the added URL', () => { + const original = new Set() + const next = addFailedImage(original, URL_A) + assert.notEqual(next, original) + assert.deepEqual([...next], [URL_A]) + assert.equal(original.size, 0) +}) + +test('addFailedImage returns the same set when the URL is already present', () => { + const original = new Set([URL_A]) + const next = addFailedImage(original, URL_A) + assert.equal(next, original) +}) + +test('addFailedImage preserves existing entries and does not mutate the input', () => { + const original = new Set([URL_A]) + const next = addFailedImage(original, URL_B) + assert.deepEqual([...next].sort(), [URL_B, URL_A].sort()) + assert.deepEqual([...original], [URL_A]) +})