6c2ecd053818535194c7845a899bca0ecd3c10d8 braney Thu Sep 17 17:08:10 2026 -0700 docent: preflight checks a session settings file named inside a goto: URL, refs #38252 `loadSession:` is the verb for a saved state, and preflight already checks the file it names. But loadSession: builds its own URL, so a test about db= TOGETHER with a session load has to spell the whole request out in a goto: -- and db= in front of the load is the entire bug in #38184. The settings file it names rots the same way any other fixture does, silently, because hgTracks answers a missing one with a perfectly good page. Read hgS_loadUrlName out of a goto: the way hubUrl is already read out of one. diff --git src/hg/utils/docent/tests/preflight.js src/hg/utils/docent/tests/preflight.js index a495a1ef7b9..e2640a5cc0c 100644 --- src/hg/utils/docent/tests/preflight.js +++ src/hg/utils/docent/tests/preflight.js @@ -1,240 +1,251 @@ #!/usr/bin/env node /* preflight.js [DIR] [SCRIPT...] * * Checks the things a directory of Docent tests depends on but does not contain: the * saved sessions the scripts load, the hubs they attach, the custom-track URLs they * fetch, and the server they all point at. No browser, so it runs in seconds and can be * run far more often than the suite it guards. * * It exists because of the way these tests fail without it. A saved session that has * been renamed or deleted produces no error: hgTracks answers 200 with a page titled * "Very Early Error" whose body reads "Could not find session NAME for user USER", the * page carries no track image, and every `noText:` assertion on it passes. The run goes * green while testing nothing. A hub URL that has moved behaves the same way. * * Fixtures are read out of the scripts rather than from a list kept beside them, so the * two cannot drift. A fixture named in no script is not checked, which is the point. * * Exit 0 if every fixture resolved, 1 otherwise, so a nightly run can tell "the fixtures * are gone" from "a bug came back". Without this they are the same red. * * With no script names it checks every *.docent.yaml in DIR, which is what you want * interactively. Name scripts to check only those: the nightly run passes the COMMITTED * list, because a directory also holds work in progress and a dead fixture belonging to * a script that is not running should not be reported as a problem. * * It also prints how the target is CONFIGURED, which is the other half of reading a * redirected run. `make test TARGET=hgwdev-you` against a personal sandbox can go red * for reasons that are not the code: a different curatedHubPrefix changes what a * quickLift hop produces, and a db.trackDb with a private table in front of the shared * one changes what `exact: true` counts. Those failures look exactly like a bug, and * telling the two apart has cost an hour more than once. So when the target is served * from THIS machine the settings that decide it are printed beside the fixtures, and the * log then carries its own explanation. * * Only the handful of settings named in HG_CONF_KEYS is ever printed. hg.conf includes * hg.conf.private, which holds database passwords, so the reader below parses whatever * the includes lead to and prints nothing that is not on that list. */ 'use strict'; const fs = require('fs'); const path = require('path'); const os = require('os'); const yaml = require('js-yaml'); // Shared with docent.js, which is the point: this program checks the fixtures for the // server the run will actually drive, so it has to resolve the target, its hg.conf, its // hgcentral and its account exactly the way the run will. const { serverFor, hgConfFor, readHgConf, centralDbFor, loginLookup } = require('../targetConf.js'); const DIR = process.argv[2] || '.'; const enc = encodeURIComponent; // The settings that change what a test sees, and nothing else. central.db says which // hgcentral the named sessions above were looked for in; db.trackDb which trackDb tables // the track names resolve against; curatedHubPrefix which curated hubs are attached; the // browser.* three whether the features several scripts here depend on are even on. const HG_CONF_KEYS = ['central.db', 'db.trackDb', 'curatedHubPrefix', 'browser.quickLift', 'browser.quickLiftAlignments', 'browser.recTrackSets']; const PAD = ' '; function reportTargetConf(server) { console.log(` target ${server}`); const file = hgConfFor(server); if (!file) { console.log(`${PAD}not on this machine, so its hg.conf cannot be read from here`); return; } if (!fs.existsSync(file)) { console.log(`${PAD}${file} -- no such file`); return; } const conf = readHgConf(file); const w = Math.max(...HG_CONF_KEYS.map(k => k.length)); console.log(`${PAD}${file}`); for (const k of HG_CONF_KEYS) console.log(`${PAD}${k.padEnd(w)} ${conf.has(k) ? conf.get(k) : 'unset'}`); } // What to print for the account a `login:` step will use on this server, and why it // cannot be used if it cannot. The lookup itself is in targetConf.js, shared with the run. // No login is attempted: a wrong password fails loudly at the step itself, which is the // one thing this program cannot do for it. No password is printed, here or anywhere. function loginAccount(server) { const c = loginLookup(server); const where = c.central ? `central.db ${c.central}` : 'central.db unknown'; if (c.why) return { label: where, why: c.why }; const from = c.source === 'the environment' ? 'the environment' : `[${c.section}]`; return { label: `${c.user} (from ${from}, ${where})`, why: null }; } const fixtures = []; const add = f => fixtures.push(f); async function get(url) { const r = await fetch(url, { redirect: 'follow' }); return { status: r.status, body: await r.text() }; } // A session is present when the page does NOT carry hgSession's own not-found message. // Requiring a track image instead would be wrong: a session may legitimately restore a // view with every track hidden. function sessionCheck(server, user, name) { const url = `${server}/hgTracks?hgS_doOtherUser=submit` + `&hgS_otherUserName=${enc(user)}&hgS_otherUserSessionName=${enc(name)}`; return async () => { const { status, body } = await get(url); if (status !== 200) return `HTTP ${status}`; // Match the stem of the message rather than rebuilding the whole string, which would // depend on how the name was escaped on the way in. if (/Could not find session/i.test(body)) return 'no such session on this server'; if (/<TITLE>\s*Very Early Error/i.test(body)) return 'server returned an early error'; return null; }; } function urlCheck(url, wantText) { return async () => { const { status, body } = await get(url); if (status !== 200) return `HTTP ${status}`; if (!body.trim()) return 'empty response'; if (wantText && !body.includes(wantText)) return `no "${wantText}" in the response`; return null; }; } const fileCheck = file => async () => fs.existsSync(file) ? null : 'no such file'; const named = process.argv.slice(3) .map(a => a.endsWith('.docent.yaml') ? a : `${a}.docent.yaml`); const scripts = (named.length ? named : fs.readdirSync(DIR).filter(f => f.endsWith('.docent.yaml'))).sort(); const seenServer = new Map(); for (const f of scripts) { let doc; try { doc = yaml.load(fs.readFileSync(path.join(DIR, f), 'utf8')) || {}; } catch (e) { add({ script: f, kind: 'script', label: f, check: async () => `unreadable YAML: ${e.message}` }); continue; } // DOCENT_TARGET redirects the run, so the server fixture has to be the one that will // actually be driven rather than the one the script names. serverFor() is the same // function docent.js uses, so the two cannot drift apart. const server = serverFor(doc.target); if (!seenServer.has(server)) seenServer.set(server, f); const base = path.basename(f, '.docent.yaml'); for (const step of (doc.steps || [])) { if (!step || typeof step !== 'object') continue; const verb = Object.keys(step)[0]; const arg = step[verb]; const o = (arg && typeof arg === 'object') ? arg : null; if (verb === 'loadSession') { if (o && o.user && o.name) { add({ script: f, kind: 'session', label: `${o.user}/${o.name}`, check: sessionCheck(server, o.user, o.name) }); } else if (o && o.file) { // A session: step earlier in the same script writes this, so it is only a // fixture when nothing here creates it. const writes = (doc.steps || []).some(s => s && typeof s === 'object' && s.session === o.file); if (!writes) { add({ script: f, kind: 'file', label: o.file, check: fileCheck(path.join(DIR, 'sessions', base, `${o.file}.txt`)) }); } } else if (typeof arg === 'string' && /^https?:/.test(arg)) { add({ script: f, kind: 'session-url', label: arg, check: urlCheck(arg) }); } } else if (verb === 'login') { // Keyed by server, so a directory pointed at two of them reports two accounts. const acct = loginAccount(server); add({ script: f, kind: 'login', label: `${acct.label} on ${server}`, check: async () => acct.why }); } else if (verb === 'goto' && typeof arg === 'string') { // A hub can also arrive inside a goto: URL, which is the only way to write a test // about the genome= form (the hub: verb builds db=). Pull hubUrl out of the query // so those hubs are checked too, rather than being invisible to preflight because // of how the step happens to be spelled. const m = /[?&]hubUrl=([^&]+)/.exec(arg); if (m) { const url = decodeURIComponent(m[1]); add({ script: f, kind: 'hub', label: url, check: urlCheck(url, 'hub') }); } + // And so can a session settings file. `loadSession:` is the verb for one, but it + // builds the URL itself, so a test about db= TOGETHER WITH a session load has to + // spell the whole thing out in a goto: -- and the settings file it names rots the + // same way any other fixture does, silently, because hgTracks answers a missing one + // with a perfectly good page. + const sm = /[?&]hgS_loadUrlName=([^&]+)/.exec(arg); + if (sm) { + const url = decodeURIComponent(sm[1]); + if (/^https?:/.test(url)) + add({ script: f, kind: 'session-url', label: url, check: urlCheck(url) }); + } } else if (verb === 'hub' || verb === 'addHub') { const url = (typeof arg === 'string') ? arg : (o && o.url); // A hub.txt replaced by a directory listing or an error page still answers 200, so // require the one word every hub.txt has to contain. if (url) add({ script: f, kind: 'hub', label: url, check: urlCheck(url, 'hub') }); } else if (verb === 'addCustomTrack') { if (o && o.url) add({ script: f, kind: 'ct-url', label: o.url, check: urlCheck(o.url) }); const rel = o && (o.file || o.pasteFile); if (rel) add({ script: f, kind: 'file', label: rel, check: fileCheck(path.join(DIR, rel)) }); } } } for (const [server, f] of seenServer) { fixtures.unshift({ script: f, kind: 'server', label: server, check: urlCheck(`${server}/hgTracks?db=hg38&pix=800`) }); } (async () => { // Before the fixtures, because it is what a red line further down has to be read // against. Costs no network and no browser. for (const server of seenServer.keys()) reportTargetConf(server); if (!fixtures.length) { console.log(`preflight: ${scripts.length} script(s), no external fixtures to check`); process.exit(0); } // One fixture named by several scripts is one check, reported against all of them. const byKey = new Map(); for (const fx of fixtures) { const k = `${fx.kind} ${fx.label}`; if (!byKey.has(k)) byKey.set(k, { ...fx, scripts: [] }); byKey.get(k).scripts.push(fx.script); } const all = [...byKey.values()]; const results = await Promise.all(all.map(async fx => { let why; try { why = await fx.check(); } catch (e) { why = `unreachable: ${e.message}`; } return { ...fx, why }; })); results.sort((a, b) => a.kind.localeCompare(b.kind) || a.label.localeCompare(b.label)); for (const r of results) { console.log(` ${(r.why ? 'MISSING' : 'ok').padEnd(8)}${r.kind.padEnd(12)}${r.label}`); if (r.why) { console.log(` ${r.why} -- needed by ${[...new Set(r.scripts)].join(', ')}`); } } const bad = results.filter(r => r.why).length; console.log(`preflight: ${all.length} fixture(s) for ${scripts.length} script(s), ${bad} missing`); process.exit(bad ? 1 : 0); })();