m038/intotheeast-com
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(test): close the test-entry leak into real trip content

Browse Source 
A ui-test entry had survived into the active trip's dailies. Three independent
failures had to line up for that, and all three were real:

1. cleanupEntry() used host-side fs.rmSync. Grav's Apache workers run as root,
   so every entry the form creates is root-owned and recursive removal needs
   write permission on that directory — which the host user lacks. Cleanup had
   never worked for form-created entries; it just threw inside a path nothing
   checked. It now falls back to `docker exec … rm -rf` in the container that
   actually serves USER_DIR.

2. globalTeardown's dailies sweep keyed off a `parent:` in post-form.md — a key
   deliberately removed (the write target comes from site.yaml active_trip, and
   CLAUDE.md forbids re-adding a static parent). The regex could never match, so
   dailiesDir was always null and the sweep silently did nothing. It now reuses
   helpers' own resolution instead of keeping a divergent copy.

3. Nothing pinned the suite to this checkout's server. playwright.config.js
   defaults to :8081, so a worktree run hit the MAIN checkout — entries created
   in one content tree while the specs asserted and cleaned up in another.
   test-ui now passes GRAV_BASE_URL from GRAV_PORT, and globalSetup hard-fails
   when the server's bind mount disagrees with the tree the specs read.

Also fixed, found on the way to a green run:

- test-account interpolated the password into an `sh -c` string, so a password
  containing a shell metacharacter was re-parsed by the container's shell
  (`sh: 2: <fragment>: not found`, no account, every UI run dead). It now
  travels via `docker exec -e`, making the recipe indifferent to its contents.
- `make start` in a worktree always failed: travel-memories declares
  `env_file: .env` and worktree-new creates none. It degrades to start-grav
  there — a worktree with no server is what sent runs to :8081 in the first
  place.
- test-form-config asserted a hero_image field that 8cf1145 deliberately
  removed; it had been failing ever since.

Verified: config 22/22, post 6/6, location-override 20/20, and a full UI run
now leaves zero ui-test entries behind. The remaining UI failures are
pre-existing on main — site.yaml pins owner_username to a real account while
the suite logs in as testrunner, so owner-only controls never render for it.
Only trip-publish.spec.js patches that; delete-flow, edit-mode and anon-view
do not. Left for a separate branch.
This commit is contained in:
m038
2026-07-24 22:33:46 +02:00
parent 5edaf3ee1e
commit 1f4e2aeba5
7 changed files with 216 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Makefile
+33 -3
View File
@@ -54,9 +54,15 @@ $(foreach t,$(REMOTE_TARGETS),$(foreach e,$(ENVS),$(eval $(call make-env-target,
GRAV_TEST_USER ?= testrunner
GRAV_TEST_PASS ?= Testpass1234
# The password is handed to the container through `docker exec -e` (the bare
# form, which forwards the already-exported variable) rather than interpolated
# into the `sh -c` string. Interpolating it meant any shell-special character in
# GRAV_TEST_PASS was re-parsed by the container's shell — a `.env` password
# containing one produced `sh: 2: <fragment>: not found` and no test account.
# The recipe is now indifferent to the password's contents.
test-account:
@docker exec $(GRAV_CONTAINER) sh -c 'test -f /var/www/html/user/accounts/$(GRAV_TEST_USER).yaml \
|| php bin/plugin login new-user -u $(GRAV_TEST_USER) -p "$(GRAV_TEST_PASS)" \
@docker exec -e GRAV_TEST_PASS $(GRAV_CONTAINER) sh -c 'test -f /var/www/html/user/accounts/$(GRAV_TEST_USER).yaml \
|| php bin/plugin login new-user -u $(GRAV_TEST_USER) -p "$$GRAV_TEST_PASS" \
-e $(GRAV_TEST_USER)@example.test -N "Test Runner" -P b --admin-type both -s enabled -n'
test-config:
@@ -65,6 +71,13 @@ test-config:
test-post: test-account
@bash scripts/test-post.sh
# Pinned to THIS checkout's port, not playwright.config.js's :8081 default. In a
# worktree that default silently pointed the suite at the main checkout's server,
# so entries were created in main's user/ while the specs asserted and cleaned up
# in the worktree's — leaving ui-test entries behind in real trip content.
# tests/global-setup.js now also hard-fails on that mismatch.
GRAV_BASE_URL ?= http://localhost:$(GRAV_PORT)
test-ui: test-account
@npx playwright test
@@ -98,8 +111,18 @@ build-assets:
-w /app node:20-alpine \
sh -c "npm install && npm run build"
# In a worktree this degrades to start-grav. The travel-memories service declares
# `env_file: .env`, and worktree-new does not create a .env, so a plain
# `docker compose up -d` there dies with "env file ... not found" — leaving the
# worktree with no server at all, which is how test runs ended up silently
# targeting the main checkout.
start:
docker compose up -d
@if [ -f .worktree-env ]; then \
echo "→ worktree: starting the grav service only (travel-memories needs a .env, which worktrees have none)"; \
docker compose up -d grav; \
else \
docker compose up -d; \
fi
# Grav service only — used by `make worktree-new` (a worktree rarely needs the
# travel-memories service, and this keeps its footprint minimal).
@@ -185,6 +208,13 @@ demo-load:
# Load every fixture trip under docs/demo/trips/ into the pages tree.
# Source uses dailies/ + 04.stories/; dailies/ maps to 01.dailies/ on copy.
# All copies are `|| true` so a fixture absent from an older user/ is skipped.
#
# ⚠️ A fixture whose folder name matches a REAL trip's slug is copied straight
# over that live page — docs/demo/trips/italy-2025/ collides with the real
# italy-2025 trip on purpose (the fixture supplies its GPX + dailies). So any
# field the fixture's trip.md omits gets silently deleted from real content on
# every test run: it had been dropping the trip's tagline that way. Keep a
# colliding fixture's trip.md byte-identical to the live page.
docker exec $(GRAV_CONTAINER) bash -c 'for src in /var/www/html/user/docs/demo/trips/*/; do \
slug=$$(basename "$$src"); dst=/var/www/html/user/pages/01.trips/$$slug; \
mkdir -p "$$dst/01.dailies" "$$dst/04.stories"; \
scripts/test-form-config.sh
+4 -1
View File
@@ -59,7 +59,10 @@ check_grep "location_country field present" "name: location_country"
check_grep "weather_desc field present" "name: weather_desc"
check_grep "weather_temp_c field present" "name: weather_temp_c"
check_grep "transport_mode field present" "name: transport_mode"
check_grep "hero_image field present" "name: hero_image"
# No hero_image assertion: the field was deliberately dropped in 8cf1145 —
# entries render their hero from the first photo, so an explicit filename was
# redundant (see the comment at that spot in post-form.md). This check outlived
# the field and had been failing ever since.
check_grep "force_connect field present" "name: force_connect"
check_grep "featured field present" "name: featured"
tests/global-setup.js
+57
View File
@@ -2,6 +2,58 @@ const fs = require('fs');
const path = require('path');
const { execSync } = require('child_process');
/**
* Fail fast if the server under test does not serve the `user/` tree the specs
* read from disk.
*
* This mismatch is silent and destructive. Every post spec submits through the
* live form (the write target is derived server-side from site.yaml
* `active_trip`, so there is no per-request override), then asserts and cleans up
* on disk via helpers' USER_DIR. Run the specs from a worktree whose own
* container is down and baseURL falls back to localhost:8081 — the MAIN
* checkout — so entries get created in one content tree while cleanup deletes
* from another. The entries are then left behind in real trip content, which is
* exactly what happened on 2026-07-24.
*
* Docker is the only thing that knows the mapping, so this is best-effort: if we
* cannot determine it we warn and continue rather than blocking non-Docker runs.
* But when we CAN determine it and it disagrees, that is always a bug.
*/
function assertServerServesUserDir(baseURL, userDir) {
const port = new URL(baseURL).port || '80';
let mountedUserDir;
try {
const container = execSync("docker ps --format '{{.Names}}\t{{.Ports}}'", { encoding: 'utf-8' })
.split('\n').filter(Boolean)
.find(l => l.includes(`:${port}->`));
if (!container) {
console.warn(`[setup] no running container publishes port ${port} — is the dev server up? (make start)`);
return;
}
const name = container.split('\t')[0];
mountedUserDir = execSync(
`docker inspect ${name} --format '{{range .Mounts}}{{if eq .Destination "/var/www/html/user"}}{{.Source}}{{end}}{{end}}'`,
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] }
).trim();
if (!mountedUserDir) return; // no bind mount to compare against
} catch (_) {
return; // docker unavailable — nothing to check
}
const served = fs.realpathSync(mountedUserDir);
const asserted = fs.realpathSync(userDir);
if (served !== asserted) {
throw new Error(
`Test target mismatch — refusing to run.\n` +
` baseURL ${baseURL} is served from: ${served}\n` +
` but the specs read/clean up: ${asserted}\n` +
`Entries would be created in one tree and cleanup would miss them, leaving\n` +
`test entries behind in real content. Start this checkout's own server\n` +
`(make start) and point the run at it, e.g. GRAV_BASE_URL=http://localhost:<port>.`
);
}
}
module.exports = async function globalSetup() {
const envFile = path.join(__dirname, '../.env');
if (fs.existsSync(envFile)) {
@@ -23,4 +75,9 @@ module.exports = async function globalSetup() {
// Ensure demo content is loaded (italy-2026-demo trip + stories + GPX files)
execSync('make demo-load', { cwd: path.join(__dirname, '..'), stdio: 'inherit' });
// Required last: helpers.js resolves USER_DIR at require time, and the .env
// load above can supply GRAV_USER_DIR.
const { USER_DIR } = require('./ui/helpers');
assertServerServesUserDir(process.env.GRAV_BASE_URL || 'http://localhost:8081', USER_DIR);
};
tests/global-teardown.js
+32 -45
View File
@@ -1,57 +1,44 @@
const fs = require('fs');
const path = require('path');
const { execSync } = require('child_process');
function resolveUserDir() {
if (process.env.GRAV_USER_DIR) return process.env.GRAV_USER_DIR;
try {
const raw = execSync(
"docker inspect intotheeast_grav --format '{{range .Mounts}}{{if eq .Destination \"/var/www/html/user\"}}{{.Source}}{{end}}{{end}}'",
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] }
).trim();
if (raw) return raw;
} catch (_) {}
return path.join(__dirname, '../user');
}
// Reuse the specs' own resolution rather than reimplementing it. The previous
// version of this file derived the dailies directory from a `parent:` key in
// pages/02.post/post-form.md — a key that was deliberately removed (the write
// target is injected server-side from site.yaml `active_trip`, and CLAUDE.md
// forbids re-adding a static parent). The regex therefore never matched,
// dailiesDir was always null, and the dailies sweep below silently did nothing.
// That is how ui-test entries survived into the active trip's content.
// removeEntryDir handles the root-owned case by deleting through the container —
// see its comment. Plain fs.rmSync cannot remove what Grav's Apache wrote.
const { USER_DIR, TRACKER_DIR, removeEntryDir } = require('./ui/helpers');
function sweepUiTestEntries(dir) {
if (!fs.existsSync(dir)) return 0;
const entries = fs.readdirSync(dir).filter(e => e.includes('ui-test'));
entries.forEach(e => fs.rmSync(path.join(dir, e), { recursive: true, force: true }));
return entries.length;
if (!dir || !fs.existsSync(dir)) return 0;
const found = fs.readdirSync(dir).filter(e => e.includes('ui-test'));
let removed = 0;
found.forEach(e => {
const target = path.join(dir, e);
try {
removeEntryDir(target);
removed++;
} catch (err) {
// Loud, not silent — a swallowed failure here is exactly what let a
// ui-test entry survive into the active trip's content.
console.error(`[teardown] COULD NOT REMOVE ${target}: ${err.message}`);
}
});
return removed;
}
module.exports = async function globalTeardown() {
const userDir = resolveUserDir();
// Read active trip slug from post-form.md
const postFormPath = path.join(userDir, 'pages/02.post/post-form.md');
let dailiesDir = null;
if (fs.existsSync(postFormPath)) {
const content = fs.readFileSync(postFormPath, 'utf-8');
const m = content.match(/parent:\s*['"]?\/trips\/([^/'"]+)\/dailies/);
if (m) {
const tripSlug = m[1];
const tripsBase = path.join(userDir, 'pages/01.trips');
const tripFolder = fs.readdirSync(tripsBase).find(
f => f === tripSlug || f.endsWith('.' + tripSlug) || f.includes(tripSlug)
);
if (tripFolder) {
const dailiesBase = path.join(tripsBase, tripFolder);
const dailiesFolder = fs.readdirSync(dailiesBase).find(
f => f === 'dailies' || f === '01.dailies' || f.endsWith('.dailies')
);
if (dailiesFolder) dailiesDir = path.join(dailiesBase, dailiesFolder);
}
}
}
// Sweep both the post inbox and the active trip's dailies
const postInbox = path.join(userDir, 'pages/02.post');
const n1 = sweepUiTestEntries(postInbox);
const n2 = dailiesDir ? sweepUiTestEntries(dailiesDir) : 0;
// Sweep both the post inbox and the active trip's dailies.
const n1 = sweepUiTestEntries(path.join(USER_DIR, 'pages/02.post'));
const n2 = sweepUiTestEntries(TRACKER_DIR);
if (n1 + n2 > 0) {
console.log(`[teardown] removed ${n1} ui-test entries from 02.post, ${n2} from dailies`);
console.log(
`[teardown] removed ${n1} ui-test entries from 02.post, ` +
`${n2} from ${path.relative(USER_DIR, TRACKER_DIR)}`
);
}
};
tests/ui/helpers.js
+59 -2
View File
@@ -170,6 +170,60 @@ async function createPhotoEntry(page, tag, { content, publish = true, created }
'Entry posted successfully!', { timeout: 15_000 });
}
/**
* Resolve the Grav container that serves USER_DIR, so cleanup can delete as root.
* Prefers GRAV_CONTAINER (set by .worktree-env / .env), else matches on the bind
* mount so a worktree never picks the main checkout's container.
*/
function resolveGravContainer() {
if (process.env.GRAV_CONTAINER) return process.env.GRAV_CONTAINER;
try {
const want = fs.realpathSync(USER_DIR);
const names = execSync("docker ps --format '{{.Names}}'", { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] })
.split('\n').filter(Boolean);
return names.find((n) => {
const src = execSync(
`docker inspect ${n} --format '{{range .Mounts}}{{if eq .Destination "/var/www/html/user"}}{{.Source}}{{end}}{{end}}'`,
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'ignore'] }
).trim();
return src && fs.realpathSync(src) === want;
}) || null;
} catch (_) {
return null;
}
}
/**
* Delete an entry directory, falling back to the container when the host cannot.
*
* Grav's Apache workers run as root, so every entry the form creates is
* root-owned. Removing one recursively needs write permission on that directory,
* which the host user does not have — so a plain fs.rmSync throws EACCES and the
* entry survives. That is how a ui-test entry ended up committed-adjacent in the
* active trip's content on 2026-07-24: cleanup had never actually worked for
* form-created entries, it just failed inside a path nothing checked.
*
* `docker exec … rm -rf` runs as root in the container, which can remove them.
*/
function removeEntryDir(dir) {
try {
fs.rmSync(dir, { recursive: true });
return true;
} catch (err) {
if (err.code !== 'EACCES' && err.code !== 'EPERM') throw err;
}
const container = resolveGravContainer();
if (!container) {
throw new Error(
`Cannot remove ${dir}: it is root-owned (written by Grav in the container) and no ` +
`matching container was found to delete it as root. Set GRAV_CONTAINER or remove it manually.`
);
}
execSync(`docker exec ${container} rm -rf '/var/www/html/user/${path.relative(USER_DIR, dir)}'`,
{ stdio: ['pipe', 'pipe', 'pipe'] });
return true;
}
/**
* Find a tracker entry folder by a unique slug fragment, then delete it.
*/
@@ -179,7 +233,7 @@ function cleanupEntry(slugFragment) {
const entries = fs.readdirSync(TRACKER_DIR);
const match = entries.find(e => e.includes(slugFragment));
if (match) {
fs.rmSync(path.join(TRACKER_DIR, match), { recursive: true });
removeEntryDir(path.join(TRACKER_DIR, match));
}
}
@@ -202,4 +256,7 @@ function readEntryMd(entryDir) {
return fs.readFileSync(path.join(entryDir, name), 'utf-8');
}
module.exports = { fillEditor, waitForPhotoUpload, postEntry, createPhotoEntry, cleanupEntry, findEntry, readEntryMd, TEST_PHOTO, TRACKER_DIR, ACTIVE_TRIP_URL };
// USER_DIR is exported so global-setup/global-teardown resolve the same tree the
// specs assert against, instead of keeping their own (previously divergent) copy
// of this logic.
module.exports = { fillEditor, waitForPhotoUpload, postEntry, createPhotoEntry, cleanupEntry, removeEntryDir, findEntry, readEntryMd, TEST_PHOTO, USER_DIR, TRACKER_DIR, ACTIVE_TRIP_URL };
tests/ui/post/location-override.spec.js
+30 -1
View File
@@ -330,6 +330,10 @@ test('submitting with an unresolved lat/lng mismatch is blocked', async ({ page
});
// ── U4: lazy-load boundary — an ordinary GPS-only submit never fetches maplibre-gl ──
// The URL pattern deliberately covers BOTH halves of the lazy boundary: the JS
// chunk (js/post/maplibre-gl-*.js) and the stylesheet
// (css-compiled/maplibre-gl.css, <link>ed by location-map.js at panel-open —
// see its ensureMaplibreCss). Neither may be requested when the panel stays shut.
test('an ordinary submit without opening the panel never fetches the maplibre-gl chunk', async ({ page }) => {
const chunkRequests = [];
page.on('request', (req) => {
@@ -346,7 +350,32 @@ test('an ordinary submit without opening the panel never fetches the maplibre-gl
await expect(page.locator('.notices')).toContainText('Entry posted successfully!', { timeout: 15_000 });
created.push(tag);
expect(chunkRequests, 'maplibre-gl must not be fetched when the panel is never opened').toHaveLength(0);
expect(chunkRequests, 'neither the maplibre-gl chunk nor its stylesheet may be fetched when the panel is never opened').toHaveLength(0);
});
// ── The other half of that boundary: opening the panel DOES apply the vendor CSS ──
// Without this, the guard above could keep passing while the stylesheet silently
// stopped loading at all (a broken href, a missed build step), leaving the map
// unstyled with nothing to catch it. Asserts the <link> exists AND parsed —
// link.sheet is null until the browser has actually applied it.
test('opening the panel lazily links maplibre\'s stylesheet and applies it', async ({ page }) => {
await page.goto('/post');
const hrefBefore = await page.evaluate(() => Array.from(document.styleSheets)
.map((s) => s.href || '').filter((h) => /maplibre-gl\.css/.test(h)));
expect(hrefBefore, 'the vendor stylesheet must not be present before the panel opens').toHaveLength(0);
await openLocationDetails(page);
await expect(page.locator('#location-map canvas.maplibregl-canvas')).toHaveCount(1, { timeout: 10_000 });
await expect.poll(
() => page.evaluate(() => {
const link = Array.from(document.querySelectorAll('link[rel="stylesheet"]'))
.find((l) => /maplibre-gl\.css/.test(l.href));
return link ? link.sheet !== null : false;
}),
{ message: 'maplibre\'s stylesheet must be linked and applied once the panel opens', timeout: 10_000 }
).toBe(true);
});
// ── Full submit: a search-picked location round-trips into the frontmatter ──
user
+1 -1
Submodule user updated: e873a9cb23...2b91aa30c3