From d576487886199cc3024d0c3a0b7d321919ca062e Mon Sep 17 00:00:00 2001 From: Mischa Date: Tue, 7 Jul 2026 08:32:09 +0200 Subject: [PATCH] test(post): share createPhotoEntry helper and fix cleanup leak Hoist the duplicated per-spec createEntry photo-fixture into a single createPhotoEntry() in helpers.js (used by delete-flow, edit-mode, and the anon-view draft). Register the tag for cleanup BEFORE the awaited 15s success-toast assertion, so a create that lands on disk but whose toast assertion times out no longer leaks an untracked entry. Add AE3b covering the disclosure deviation branch (a non-default toggle auto-expands More options). Code review F2 (leak), F3 (duplication), F6 (coverage). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Mpdu3Dt1iVoozHwAMyjrbn --- tests/ui/helpers.js | 38 +++++++++++++++++++++++++++++- tests/ui/post/anon-view.spec.js | 24 ++++--------------- tests/ui/post/delete-flow.spec.js | 26 ++++---------------- tests/ui/post/edit-mode.spec.js | 19 ++------------- tests/ui/post/post-form-ux.spec.js | 26 ++++++++++++++++++++ 5 files changed, 74 insertions(+), 59 deletions(-) diff --git a/tests/ui/helpers.js b/tests/ui/helpers.js index b397ad1..a825606 100644 --- a/tests/ui/helpers.js +++ b/tests/ui/helpers.js @@ -2,6 +2,11 @@ const path = require('path'); const fs = require('fs'); const { execSync } = require('child_process'); +const { expect } = require('@playwright/test'); + +// The shared photo fixture every create goes through (the post form gates submit +// on at least one uploaded photo). +const TEST_PHOTO = path.join(__dirname, '../fixtures/test-photo.jpg'); /** * Resolve the Grav user directory. @@ -134,6 +139,37 @@ async function postEntry(page, { titleTag, content = 'Automated test. Safe to de return titleTag; } +/** + * Create a fresh journal entry through the /post create form, with a photo + * attached so the submit gate is satisfied. Shared by the specs that need a + * disposable feed card to act on (delete-flow, edit-mode, anon-view draft). + * + * Pass the spec's `created` array so the tag is registered for cleanup BEFORE + * the (slow, 15s) success-toast assertion — a create that lands on disk but + * whose toast assertion times out would otherwise leak an entry the afterAll + * hook never sees. `publish:false` flips the Published toggle off to make a + * draft (the toggle is a visually-hidden radio pair behind "More options", so + * set state + fire `change` rather than fighting the visibility gate). + */ +async function createPhotoEntry(page, tag, { content, publish = true, created } = {}) { + await page.goto('/post'); + await page.fill('input[name="data[title]"]', `UI Test ${tag}`); + await fillEditor(page, content || `Fixture for ${tag}. Safe to delete.`); + await page.locator('input.filepond--browser').setInputFiles(TEST_PHOTO); + await waitForPhotoUpload(page); + if (!publish) { + await page.evaluate(() => { + const off = document.querySelector('input[name="data[published]"][value="0"]'); + off.checked = true; + off.dispatchEvent(new Event('change', { bubbles: true })); + }); + } + await page.locator('.btn-post').evaluate(el => el.click()); + if (created) created.push(tag); + await expect(page.locator('.form-messages, .notices')).toContainText( + 'Entry posted successfully!', { timeout: 15_000 }); +} + /** * Find a tracker entry folder by a unique slug fragment, then delete it. */ @@ -166,4 +202,4 @@ function readEntryMd(entryDir) { return fs.readFileSync(path.join(entryDir, name), 'utf-8'); } -module.exports = { fillEditor, waitForPhotoUpload, postEntry, cleanupEntry, findEntry, readEntryMd, TRACKER_DIR, ACTIVE_TRIP_URL }; +module.exports = { fillEditor, waitForPhotoUpload, postEntry, createPhotoEntry, cleanupEntry, findEntry, readEntryMd, TEST_PHOTO, TRACKER_DIR, ACTIVE_TRIP_URL }; diff --git a/tests/ui/post/anon-view.spec.js b/tests/ui/post/anon-view.spec.js index 443c5db..e5b929d 100644 --- a/tests/ui/post/anon-view.spec.js +++ b/tests/ui/post/anon-view.spec.js @@ -13,10 +13,8 @@ // short-lived authenticated context to create the draft fixture and confirm the // owner-visible side. const { test, expect } = require('@playwright/test'); -const path = require('path'); -const { fillEditor, waitForPhotoUpload, cleanupEntry, findEntry, ACTIVE_TRIP_URL } = require('../helpers'); +const { createPhotoEntry, cleanupEntry, findEntry, ACTIVE_TRIP_URL } = require('../helpers'); -const TEST_PHOTO = path.join(__dirname, '../../fixtures/test-photo.jpg'); const AUTH_STATE = 'tests/.auth/user.json'; const BASE = process.env.GRAV_BASE_URL || 'http://localhost:8081'; @@ -47,23 +45,11 @@ test('AN2: a draft entry is shown to the owner but hidden from an anonymous visi // Create an UNPUBLISHED entry as the owner, in a separate authed context. const owner = await browser.newContext({ storageState: AUTH_STATE, baseURL: BASE }); const op = await owner.newPage(); - await op.goto('/post'); - await op.fill('input[name="data[title]"]', `UI Test ${tag}`); - await fillEditor(op, `Draft body ${tag}. Safe to delete.`); - await op.locator('input.filepond--browser').setInputFiles(TEST_PHOTO); - await waitForPhotoUpload(op); - // Flip Published → No. The toggle renders as a visually-hidden radio pair - // behind "More options", so set the state directly and fire `change` (what a - // real click would do) rather than fighting the visibility gate. - await op.evaluate(() => { - const off = document.querySelector('input[name="data[published]"][value="0"]'); - off.checked = true; - off.dispatchEvent(new Event('change', { bubbles: true })); + await createPhotoEntry(op, tag, { + created, + publish: false, + content: `Draft body ${tag}. Safe to delete.`, }); - await op.locator('.btn-post').evaluate(el => el.click()); - await expect(op.locator('.form-messages, .notices')).toContainText( - 'Entry posted successfully!', { timeout: 15_000 }); - created.push(tag); expect(findEntry(tag), 'draft fixture should exist on disk').not.toBeNull(); // Owner side: the draft appears in the feed WITH a Draft badge. diff --git a/tests/ui/post/delete-flow.spec.js b/tests/ui/post/delete-flow.spec.js index 74fec9a..b71b4d2 100644 --- a/tests/ui/post/delete-flow.spec.js +++ b/tests/ui/post/delete-flow.spec.js @@ -8,34 +8,18 @@ // - DEL2: Cancel is a real escape hatch — nothing is deleted. // - DEL3: a failed DELETE keeps the card and surfaces the inline error. const { test, expect } = require('@playwright/test'); -const path = require('path'); const { - fillEditor, waitForPhotoUpload, cleanupEntry, findEntry, ACTIVE_TRIP_URL, + createPhotoEntry, cleanupEntry, findEntry, ACTIVE_TRIP_URL, } = require('../helpers'); -const TEST_PHOTO = path.join(__dirname, '../../fixtures/test-photo.jpg'); - const created = []; // cleanupEntry is a no-op when the entry was already deleted by the test. test.afterAll(() => created.forEach(cleanupEntry)); -/** Post a fresh entry through the create form so a feed card exists to delete. */ -async function createEntry(page, tag) { - await page.goto('/post'); - await page.fill('input[name="data[title]"]', `UI Test ${tag}`); - await fillEditor(page, `Delete-flow fixture ${tag}. Safe to delete.`); - await page.locator('input.filepond--browser').setInputFiles(TEST_PHOTO); - await waitForPhotoUpload(page); - await page.locator('.btn-post').evaluate(el => el.click()); - await expect(page.locator('.form-messages, .notices')).toContainText( - 'Entry posted successfully!', { timeout: 15_000 }); -} - // ── DEL1: happy delete removes the card and the folder ──────────────────────── test('DEL1: owner deletes an entry — the card disappears and the folder is removed', async ({ page }) => { const tag = `del1-${Date.now()}`; - await createEntry(page, tag); - created.push(tag); + await createPhotoEntry(page, tag, { created, content: `Delete-flow fixture ${tag}. Safe to delete.` }); await page.goto(ACTIVE_TRIP_URL); const card = page.locator('.journal-post', { hasText: tag }); @@ -52,8 +36,7 @@ test('DEL1: owner deletes an entry — the card disappears and the folder is rem // ── DEL2: Cancel keeps the entry ────────────────────────────────────────────── test('DEL2: cancelling the confirm step keeps the entry on the page and on disk', async ({ page }) => { const tag = `del2-${Date.now()}`; - await createEntry(page, tag); - created.push(tag); + await createPhotoEntry(page, tag, { created, content: `Delete-flow fixture ${tag}. Safe to delete.` }); await page.goto(ACTIVE_TRIP_URL); const card = page.locator('.journal-post', { hasText: tag }); @@ -71,8 +54,7 @@ test('DEL2: cancelling the confirm step keeps the entry on the page and on disk' // ── DEL3: a failed DELETE keeps the card and shows the inline error ─────────── test('DEL3: a failed delete keeps the card and surfaces the inline error', async ({ page }) => { const tag = `del3-${Date.now()}`; - await createEntry(page, tag); - created.push(tag); + await createPhotoEntry(page, tag, { created, content: `Delete-flow fixture ${tag}. Safe to delete.` }); await page.goto(ACTIVE_TRIP_URL); // Force the delete request to fail after the confirm. diff --git a/tests/ui/post/edit-mode.spec.js b/tests/ui/post/edit-mode.spec.js index 02a6771..b668cc9 100644 --- a/tests/ui/post/edit-mode.spec.js +++ b/tests/ui/post/edit-mode.spec.js @@ -9,35 +9,20 @@ // (404 "no longer exists" vs a transient "couldn't be loaded") — the copy // that tells the owner whether a retry is worthwhile. These had zero coverage. const { test, expect } = require('@playwright/test'); -const path = require('path'); const { - fillEditor, waitForPhotoUpload, cleanupEntry, findEntry, readEntryMd, ACTIVE_TRIP_URL, + fillEditor, createPhotoEntry, cleanupEntry, findEntry, readEntryMd, ACTIVE_TRIP_URL, } = require('../helpers'); -const TEST_PHOTO = path.join(__dirname, '../../fixtures/test-photo.jpg'); // Synthetic route for the mocked error tests — never has to exist on disk. const MISSING_ROUTE = '/trips/italy-2026-demo/dailies/does-not-exist'; const created = []; test.afterAll(() => created.forEach(cleanupEntry)); -/** Post a fresh entry through the create form (photo included, gate satisfied). */ -async function createEntry(page, tag) { - await page.goto('/post'); - await page.fill('input[name="data[title]"]', `UI Test ${tag}`); - await fillEditor(page, `Fixture for ${tag}. Safe to delete.`); - await page.locator('input.filepond--browser').setInputFiles(TEST_PHOTO); - await waitForPhotoUpload(page); - await page.locator('.btn-post').evaluate(el => el.click()); - await expect(page.locator('.form-messages, .notices')).toContainText( - 'Entry posted successfully!', { timeout: 15_000 }); -} - // ── ES1: edit-mode save writes the changed title + body back to disk ────────── test('ES1: editing an entry saves the changed title and body back in place', async ({ page }) => { const tag = `es1-${Date.now()}`; - await createEntry(page, tag); - created.push(tag); + await createPhotoEntry(page, tag, { created }); // Reach edit mode the way the owner does: via the feed card's Edit link. await page.goto(ACTIVE_TRIP_URL); diff --git a/tests/ui/post/post-form-ux.spec.js b/tests/ui/post/post-form-ux.spec.js index 720c5e1..992632d 100644 --- a/tests/ui/post/post-form-ux.spec.js +++ b/tests/ui/post/post-form-ux.spec.js @@ -28,6 +28,32 @@ test('AE3: advanced fields are hidden until "More options" is expanded', async ( await expect(details).toHaveJSProperty('open', true); }); +// ── AE3b: a toggle that deviates from its default auto-opens "More options" ──── +// AE3 covers the common non-deviating case (collapsed on a plain create). This +// covers the OTHER branch of initDisclosure: an advanced toggle whose value +// differs from its blueprint default force-opens the panel so a non-default +// setting is never hidden. It also guards the data-driven default detection — +// initDisclosure reads each toggle's default from the rendered `[checked]` +// attribute rather than a hardcoded field name, so this must hold for a +// default-OFF toggle (featured) flipped ON just as it does for published. +test('AE3b: a non-default advanced toggle auto-expands "More options" on load', async ({ page }) => { + await page.goto('/post'); + // Seed a create draft whose `featured` toggle deviates from its OFF default, + // then reload so initDraft restores it before initDisclosure's auto-open check. + await page.evaluate((k) => { + localStorage.setItem(k, JSON.stringify({ 'data[featured]': '1' })); + }, DRAFT_KEY); + await page.reload(); + + const details = page.locator('details.more-options'); + await expect(details).toBeAttached(); + await expect(details).toHaveJSProperty('open', true); + // The restored deviation is reflected in the live toggle state. + await expect(page.locator('input[name="data[featured]"][value="1"]')).toBeChecked(); + + await page.evaluate((k) => localStorage.removeItem(k), DRAFT_KEY); +}); + // ── AE1: HEIC → JPEG conversion, posted with a working thumbnail ─────────────── test('AE1: a HEIC photo is converted to JPEG client-side and posted', async ({ page }) => { const tag = `heic-${Date.now()}`;