From 2b37c4b24fd9cc35c51befb2c2fb192cd9308fcb Mon Sep 17 00:00:00 2001 From: "Joseph B. McQueen" Date: Wed, 1 Jul 2026 18:21:02 -0400 Subject: [PATCH] Phase 8: extract pure helpers into lib/ and cover with node:test - Move buildingKey, jobsForApp, getBotToken, isBotEnabled, getBotConfig, isAuthorized, getOAuthRedirectUri, buildAuthUrl, cleanCompletedJobs, and msToTime into lib/helpers.js as state-free functions that accept config, botTokens, or env as parameters. COMPLETED_RETENTION_DAYS also lives there so callers and tests share the constant. - Replace the bodies in index.js with thin wrappers that pass the module- level state into the pure helpers. Call sites and behavior are unchanged; index.js shrinks by ~60 lines. - Move the cleanCompletedJobs logging into the cron caller so the pure helper returns a result object (jobs, removed, cutoff) that tests can assert on without capturing stdout. - Add test/helpers.test.js with 43 assertions across 10 suites covering the enable/disable gating, per-bot draft isolation, authorization, OAuth URL construction, retention filter (including endTime -> startTime -> created fallback and the safety default for jobs missing a timestamp), and the duration formatter. - Wire `npm test` to `node --test test/*.test.js` (no new deps, uses the built-in node:test runner) and document it in the README. Smoke test confirms unchanged HTTP behavior for /info (known + unknown bots), the requireBot 404 gate, and the 401 path on jobs/list/completed. Co-authored-by: Cursor --- README.md | 17 +++ index.js | 107 ++++---------- lib/helpers.js | 118 +++++++++++++++ package.json | 2 +- test/helpers.test.js | 343 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 508 insertions(+), 79 deletions(-) create mode 100644 lib/helpers.js create mode 100644 test/helpers.test.js diff --git a/README.md b/README.md index cab8cec..f617524 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,23 @@ Runtime state (gitignored, not committed): - `config/userPrefs.json` — per-user preferences (language, etc.). - `uploads/` — CSVs of recipient IDs and any attached images. +## Testing + +Unit tests for the pure helpers extracted into `lib/helpers.js` run under +Node's built-in test runner — no additional dev dependencies required. + +``` +npm test +``` + +The suite covers bot enable/disable gating (`getBotToken`, `isBotEnabled`, +`getBotConfig`), per-bot draft isolation (`buildingKey`, `jobsForApp`), the +authorization check (`isAuthorized`), OAuth URL construction (`buildAuthUrl`, +`getOAuthRedirectUri`), the retention filter (`cleanCompletedJobs`), and the +duration formatter (`msToTime`). Adding a new helper? Add it to `lib/helpers.js` +and cover it in `test/helpers.test.js` — keeping the state-carrying wrappers +in `index.js` thin means each helper can be tested without booting the server. + ## Behavior notes - **Completed-job retention**: the daily cleanup cron (default `01:10` in diff --git a/index.js b/index.js index e2f2d15..13531af 100644 --- a/index.js +++ b/index.js @@ -12,6 +12,7 @@ import cookieParser from 'cookie-parser'; import FormData from 'form-data'; import cron from "node-cron"; import PQueue from 'p-queue'; +import * as helpers from './lib/helpers.js'; //import pLimit from "p-limit"; //const limit = pLimit(10); const queue = new PQueue({ concurrency: 10 }); @@ -44,15 +45,11 @@ function loadBotTokens() { } function getBotToken(appName) { - var entry = botTokens[appName]; - if (!entry) return null; - if (typeof entry === 'string') return entry; // tolerate flat shape - if (entry.enabled === false) return null; - return entry.token || null; + return helpers.getBotToken(botTokens, appName); } function isBotEnabled(appName) { - return getBotToken(appName) !== null; + return helpers.isBotEnabled(botTokens, appName); } // Returns the bot's config block if the bot is both defined in config.json AND @@ -60,11 +57,7 @@ function isBotEnabled(appName) { // instead of reaching into config.webex.bot[...] directly so unknown or // disabled bots can't crash the request. function getBotConfig(appName) { - if (!appName) return null; - var cfg = config.webex && config.webex.bot && config.webex.bot[appName]; - if (!cfg) return null; - if (!isBotEnabled(appName)) return null; - return cfg; + return helpers.getBotConfig(config, botTokens, appName); } // Cached bot profiles populated from Webex /people/me at startup so the @@ -123,13 +116,11 @@ function requireBot(req, res, next) { // Building (draft) jobs are keyed by cookieId AND appName so a user who is // authorized for more than one bot can have one independent draft per bot. function buildingKey(req) { - return (req.cookies && req.cookies.id) + ':' + req.params.app; + return helpers.buildingKey(req.cookies && req.cookies.id, req.params.app); } -// Filters one of the global job arrays (running/scheduled/completed) down to -// the jobs that belong to the requested bot. function jobsForApp(arr, appName) { - return (arr || []).filter(function (j) { return j && j.appName === appName; }); + return helpers.jobsForApp(arr, appName); } // Service-account OAuth tokens are rewritten in place by refreshToken(), so @@ -165,24 +156,17 @@ function getServiceAccountAccessToken() { // developer portal (one URI per bot). Returns null if required env vars are // missing so callers can return an actionable error instead of a malformed URL. function buildAuthUrl(appName) { - var clientId = process.env.WEBEX_INTEGRATION_CLIENT_ID; - var template = process.env.OAUTH_CALLBACK_URL_TEMPLATE; - if (!clientId || !template) { - logger('buildAuthUrl', 'Missing WEBEX_INTEGRATION_CLIENT_ID or OAUTH_CALLBACK_URL_TEMPLATE env var.'); - return null; - } - var params = new URLSearchParams(); - params.append('client_id', clientId); - params.append('response_type', 'code'); - params.append('redirect_uri', template.replace(':app', appName)); - params.append('scope', 'spark:kms spark:people_read'); - params.append('state', ''); - return 'https://webexapis.com/v1/authorize?' + params.toString(); + var url = helpers.buildAuthUrl({ + clientId: process.env.WEBEX_INTEGRATION_CLIENT_ID, + template: process.env.OAUTH_CALLBACK_URL_TEMPLATE, + appName: appName, + }); + if (!url) logger('buildAuthUrl', 'Missing WEBEX_INTEGRATION_CLIENT_ID or OAUTH_CALLBACK_URL_TEMPLATE env var.'); + return url; } function getOAuthRedirectUri(appName) { - var template = process.env.OAUTH_CALLBACK_URL_TEMPLATE || ''; - return template.replace(':app', appName); + return helpers.getOAuthRedirectUri(process.env.OAUTH_CALLBACK_URL_TEMPLATE, appName); } //Load Express Server @@ -1271,64 +1255,31 @@ function saveConfig(jsonObject, configFile) { }) } -// Completed jobs are kept for review for COMPLETED_RETENTION_DAYS. Anything -// older is dropped from jobs.completed by the daily cron. Bumping this number -// just means we hold more history (and a bigger jobs.json) on disk. -const COMPLETED_RETENTION_DAYS = 30; +// Retention window for completed jobs (re-exported from lib/helpers.js so +// index.js has a single symbol name callers can reason about). Bump this by +// editing helpers.js if you want a longer / shorter review window. +const COMPLETED_RETENTION_DAYS = helpers.COMPLETED_RETENTION_DAYS; async function cleanCompletedJobs(jobs) { - var cutoffDate = new Date(Date.now() - (COMPLETED_RETENTION_DAYS * 24 * 60 * 60 * 1000)); - if (!Array.isArray(jobs.completed)) { - console.log('No completed array found – nothing to do.'); - return jobs; + var result = helpers.cleanCompletedJobs(jobs, COMPLETED_RETENTION_DAYS); + if (!Array.isArray(result.jobs.completed)) { + logger('cleanCompletedJobs', 'No completed array found – nothing to do.'); + } else { + logger('cleanCompletedJobs', + 'Removed ' + result.removed + ' job(s) older than ' + COMPLETED_RETENTION_DAYS + + ' days (cutoff ' + result.cutoff.toISOString().split('T')[0] + '). Remaining: ' + + result.jobs.completed.length); } - - const originalCount = jobs.completed.length; - - jobs.completed = jobs.completed.filter(job => { - // Prefer endTime, fall back to startTime if missing - const jobDateStr = job.endTime || job.startTime || job.created; - if (!jobDateStr) return true; // safety: keep if no date - - const jobDate = new Date(jobDateStr); - return jobDate >= cutoffDate; - }); - - const removed = originalCount - jobs.completed.length; - console.log(`Removed ${removed} job(s) older than ${COMPLETED_RETENTION_DAYS} days (cutoff ${cutoffDate.toISOString().split('T')[0]}).`); - console.log(`Remaining completed jobs: ${jobs.completed.length}`); - - return jobs; + return result.jobs; } function isAuthorized(appName, personId) { logger("isAuthorized", appName + " " + personId) - var botCfg = getBotConfig(appName); - if (!botCfg) return false; - if (!personId) return false; - return !!(botCfg.authorized && botCfg.authorized[personId]); + return helpers.isAuthorized(config, botTokens, appName, personId); } function msToTime(duration) { - var milliseconds = parseInt((duration % 1000) / 100) - , seconds = parseInt((duration / 1000) % 60) - , minutes = parseInt((duration / (1000 * 60)) % 60) - , hours = parseInt((duration / (1000 * 60 * 60)) % 24); - - //hours = (hours < 10) ? "0" + hours : hours; - //minutes = (minutes < 10) ? "0" + minutes : minutes; - //seconds = (seconds < 10) ? "0" + seconds : seconds; - - var resultTime = ""; - if (hours > 0) { - resultTime = hours + "h " + minutes + "m " + seconds + "." + milliseconds + "s"; - } else if (minutes > 0) { - resultTime = minutes + "m " + seconds + "." + milliseconds + "s"; - } else { - resultTime = seconds + "." + milliseconds + "s"; - } - - return resultTime; + return helpers.msToTime(duration); } function logger(activeFunction, logLine) { diff --git a/lib/helpers.js b/lib/helpers.js new file mode 100644 index 0000000..7f93ec6 --- /dev/null +++ b/lib/helpers.js @@ -0,0 +1,118 @@ +// Pure helpers extracted from index.js. Everything here is state-free: callers +// pass in whatever piece of config, tokens, or env they want to evaluate +// against. That keeps the functions trivially unit-testable and lets the +// server module stay the single place that owns mutable runtime state. + +// Default number of days that completed jobs are retained before the daily +// cleanup cron prunes them. Exported so callers and tests share the constant. +export const COMPLETED_RETENTION_DAYS = 30; + +// Composite key for the "draft job being built" bucket. Keying by cookieId + +// appName means a user authorized on multiple bots can build one draft per +// bot without them colliding on top of each other. +export function buildingKey(cookieId, appName) { + return String(cookieId) + ':' + String(appName); +} + +// Filters one of the global job arrays (running/scheduled/completed) down to +// the jobs that belong to the requested bot. Null-safe so callers can hand +// in an uninitialized array. +export function jobsForApp(arr, appName) { + return (arr || []).filter(function (j) { return j && j.appName === appName; }); +} + +// Returns the bot's raw access token from the botTokens map, honoring the +// enabled flag. Returns null when the bot is unknown, explicitly disabled, +// or missing a token. Tolerates a flat "appName -> tokenString" shape too, +// which is what older configs used before the enabled flag was introduced. +export function getBotToken(botTokens, appName) { + if (!botTokens) return null; + var entry = botTokens[appName]; + if (!entry) return null; + if (typeof entry === 'string') return entry; + if (entry.enabled === false) return null; + return entry.token || null; +} + +export function isBotEnabled(botTokens, appName) { + return getBotToken(botTokens, appName) !== null; +} + +// Returns the bot's config block only if the bot is defined in config.json +// AND has an enabled token entry. Returns null otherwise. Every caller should +// go through this instead of reaching into config.webex.bot[...] directly, +// so unknown or disabled bots produce a clean 404 rather than a crash. +export function getBotConfig(config, botTokens, appName) { + if (!appName) return null; + var cfg = config && config.webex && config.webex.bot && config.webex.bot[appName]; + if (!cfg) return null; + if (!isBotEnabled(botTokens, appName)) return null; + return cfg; +} + +// True iff `personId` is listed under the bot's `authorized` map AND the bot +// is enabled. Missing personId or missing bot config yields false. +export function isAuthorized(config, botTokens, appName, personId) { + var botCfg = getBotConfig(config, botTokens, appName); + if (!botCfg) return false; + if (!personId) return false; + return !!(botCfg.authorized && botCfg.authorized[personId]); +} + +// Replaces the `:app` placeholder in the OAuth callback URL template with the +// appName. Empty template returns an empty string so callers can detect the +// misconfiguration. +export function getOAuthRedirectUri(template, appName) { + return (template || '').replace(':app', appName); +} + +// Builds the Webex OAuth authorize URL for a bot. Returns null when the +// required inputs (clientId / template) are missing, so callers can serve an +// actionable 500 instead of a malformed URL. +export function buildAuthUrl({ clientId, template, appName }) { + if (!clientId || !template) return null; + var params = new URLSearchParams(); + params.append('client_id', clientId); + params.append('response_type', 'code'); + params.append('redirect_uri', getOAuthRedirectUri(template, appName)); + params.append('scope', 'spark:kms spark:people_read'); + params.append('state', ''); + return 'https://webexapis.com/v1/authorize?' + params.toString(); +} + +// Drops completed jobs older than `retentionDays` from `jobs.completed`. +// Mutates `jobs` in place (matching the pre-extraction behavior) and returns +// a report so callers can log the counts without importing a logger. Falls +// back through endTime → startTime → created for the age comparison, and +// keeps any job that has no timestamp at all as a safety measure. +export function cleanCompletedJobs(jobs, retentionDays = COMPLETED_RETENTION_DAYS, now = Date.now()) { + var cutoffDate = new Date(now - (retentionDays * 24 * 60 * 60 * 1000)); + if (!Array.isArray(jobs.completed)) { + return { jobs, removed: 0, cutoff: cutoffDate, retentionDays }; + } + var originalCount = jobs.completed.length; + jobs.completed = jobs.completed.filter(function (job) { + var jobDateStr = job.endTime || job.startTime || job.created; + if (!jobDateStr) return true; + return new Date(jobDateStr) >= cutoffDate; + }); + return { + jobs, + removed: originalCount - jobs.completed.length, + cutoff: cutoffDate, + retentionDays, + }; +} + +// Formats a millisecond duration as "Nh Nm N.Ns" (or just "N.Ns" for < 1min, +// or "Nm N.Ns" for < 1hr). Matches the original stat display in job cards. +export function msToTime(duration) { + var milliseconds = parseInt((duration % 1000) / 100) + , seconds = parseInt((duration / 1000) % 60) + , minutes = parseInt((duration / (1000 * 60)) % 60) + , hours = parseInt((duration / (1000 * 60 * 60)) % 24); + + if (hours > 0) return hours + "h " + minutes + "m " + seconds + "." + milliseconds + "s"; + if (minutes > 0) return minutes + "m " + seconds + "." + milliseconds + "s"; + return seconds + "." + milliseconds + "s"; +} diff --git a/package.json b/package.json index f0e1aa0..1eb7f55 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "main": "index.js", "scripts": { "start": "node --env-file=.env index.js", - "test": "echo \"Error: no test specified\" && exit 1" + "test": "node --test test/*.test.js" }, "author": "Joseph B. McQueen", "license": "ISC", diff --git a/test/helpers.test.js b/test/helpers.test.js new file mode 100644 index 0000000..21387b1 --- /dev/null +++ b/test/helpers.test.js @@ -0,0 +1,343 @@ +import { test, describe } from 'node:test'; +import assert from 'node:assert/strict'; +import { + buildingKey, + jobsForApp, + getBotToken, + isBotEnabled, + getBotConfig, + isAuthorized, + getOAuthRedirectUri, + buildAuthUrl, + cleanCompletedJobs, + msToTime, + COMPLETED_RETENTION_DAYS, +} from '../lib/helpers.js'; + +// A minimal, self-contained config/tokens pair used across the auth tests so +// each test doesn't have to reconstruct one. Keeping novi enabled + techupdates +// disabled + noman unknown covers the three interesting bot states. +function makeState() { + return { + config: { + webex: { + bot: { + novi: { + label: 'Novi Communicator', + authorized: { + 'personA': { id: 'personA' }, + 'personB': { id: 'personB' }, + }, + }, + techupdates: { + label: 'Tech Updates', + authorized: { + 'personA': { id: 'personA' }, + }, + }, + orphan: { + label: 'Configured but no authorized users', + }, + }, + }, + }, + botTokens: { + novi: { token: 'novi-token', enabled: true }, + techupdates: { token: 'techupdates-token', enabled: false }, + orphan: { token: 'orphan-token', enabled: true }, + flatstring: 'flat-token', + }, + }; +} + +describe('buildingKey', () => { + test('composites cookieId and appName with a colon', () => { + assert.equal(buildingKey('abc123', 'novi'), 'abc123:novi'); + }); + + test('yields distinct keys for the same cookie across different bots', () => { + assert.notEqual(buildingKey('abc123', 'novi'), buildingKey('abc123', 'techupdates')); + }); + + test('stringifies an undefined cookieId (matches legacy behavior)', () => { + assert.equal(buildingKey(undefined, 'novi'), 'undefined:novi'); + }); +}); + +describe('jobsForApp', () => { + const jobs = [ + { jobId: 1, appName: 'novi' }, + { jobId: 2, appName: 'techupdates' }, + { jobId: 3, appName: 'novi' }, + { jobId: 4 }, + null, + ]; + + test('returns only jobs matching the given appName', () => { + const result = jobsForApp(jobs, 'novi'); + assert.deepEqual(result.map(j => j.jobId), [1, 3]); + }); + + test('returns empty array when appName has no jobs', () => { + assert.deepEqual(jobsForApp(jobs, 'nomatch'), []); + }); + + test('null / undefined input returns []', () => { + assert.deepEqual(jobsForApp(null, 'novi'), []); + assert.deepEqual(jobsForApp(undefined, 'novi'), []); + }); + + test('drops null entries and jobs missing appName', () => { + assert.equal(jobsForApp(jobs, 'novi').every(j => j && j.appName === 'novi'), true); + }); +}); + +describe('getBotToken', () => { + const { botTokens } = makeState(); + + test('returns the token for an enabled bot', () => { + assert.equal(getBotToken(botTokens, 'novi'), 'novi-token'); + }); + + test('returns null for an explicitly disabled bot', () => { + assert.equal(getBotToken(botTokens, 'techupdates'), null); + }); + + test('returns null for an unknown bot', () => { + assert.equal(getBotToken(botTokens, 'nomatch'), null); + }); + + test('tolerates the flat "appName -> tokenString" shape', () => { + assert.equal(getBotToken(botTokens, 'flatstring'), 'flat-token'); + }); + + test('returns null when botTokens is null or missing', () => { + assert.equal(getBotToken(null, 'novi'), null); + assert.equal(getBotToken(undefined, 'novi'), null); + }); + + test('returns null when the entry has no token field', () => { + assert.equal(getBotToken({ novi: { enabled: true } }, 'novi'), null); + }); +}); + +describe('isBotEnabled', () => { + const { botTokens } = makeState(); + + test('true for enabled bot', () => { + assert.equal(isBotEnabled(botTokens, 'novi'), true); + }); + + test('false for disabled bot', () => { + assert.equal(isBotEnabled(botTokens, 'techupdates'), false); + }); + + test('false for unknown bot', () => { + assert.equal(isBotEnabled(botTokens, 'nomatch'), false); + }); +}); + +describe('getBotConfig', () => { + const { config, botTokens } = makeState(); + + test('returns the bot config block when the bot is defined and enabled', () => { + const cfg = getBotConfig(config, botTokens, 'novi'); + assert.equal(cfg.label, 'Novi Communicator'); + }); + + test('returns null when the bot is disabled even if it is in config.json', () => { + assert.equal(getBotConfig(config, botTokens, 'techupdates'), null); + }); + + test('returns null when the bot is unknown', () => { + assert.equal(getBotConfig(config, botTokens, 'nomatch'), null); + }); + + test('returns null when appName is falsy', () => { + assert.equal(getBotConfig(config, botTokens, undefined), null); + assert.equal(getBotConfig(config, botTokens, ''), null); + }); + + test('returns null when the bot has a token but is not in config.json', () => { + // flatstring has a token entry but no matching config.webex.bot entry. + assert.equal(getBotConfig(config, botTokens, 'flatstring'), null); + }); +}); + +describe('isAuthorized', () => { + const { config, botTokens } = makeState(); + + test('true when the person is listed under the bot and the bot is enabled', () => { + assert.equal(isAuthorized(config, botTokens, 'novi', 'personA'), true); + }); + + test('false when the person is not listed under this bot', () => { + assert.equal(isAuthorized(config, botTokens, 'novi', 'stranger'), false); + }); + + test('false when the bot is disabled, even for a listed person', () => { + assert.equal(isAuthorized(config, botTokens, 'techupdates', 'personA'), false); + }); + + test('false when personId is falsy', () => { + assert.equal(isAuthorized(config, botTokens, 'novi', undefined), false); + assert.equal(isAuthorized(config, botTokens, 'novi', ''), false); + }); + + test('false when the bot config has no authorized block', () => { + assert.equal(isAuthorized(config, botTokens, 'orphan', 'personA'), false); + }); +}); + +describe('getOAuthRedirectUri', () => { + test('replaces the :app placeholder with the appName', () => { + assert.equal( + getOAuthRedirectUri('https://bot.example.com/CollabCentral/:app/oauth', 'novi'), + 'https://bot.example.com/CollabCentral/novi/oauth', + ); + }); + + test('returns empty string when the template is missing', () => { + assert.equal(getOAuthRedirectUri(undefined, 'novi'), ''); + assert.equal(getOAuthRedirectUri('', 'novi'), ''); + }); + + test('leaves the template unchanged when :app is absent', () => { + assert.equal( + getOAuthRedirectUri('https://bot.example.com/callback', 'novi'), + 'https://bot.example.com/callback', + ); + }); +}); + +describe('buildAuthUrl', () => { + const validInputs = { + clientId: 'test-client-id', + template: 'https://bot.example.com/CollabCentral/:app/oauth', + appName: 'novi', + }; + + test('returns a well-formed Webex authorize URL when both env inputs are present', () => { + const url = new URL(buildAuthUrl(validInputs)); + assert.equal(url.origin + url.pathname, 'https://webexapis.com/v1/authorize'); + assert.equal(url.searchParams.get('client_id'), 'test-client-id'); + assert.equal(url.searchParams.get('response_type'), 'code'); + assert.equal( + url.searchParams.get('redirect_uri'), + 'https://bot.example.com/CollabCentral/novi/oauth', + ); + assert.equal(url.searchParams.get('scope'), 'spark:kms spark:people_read'); + }); + + test('substitutes appName into the redirect_uri per bot', () => { + const noviUrl = new URL(buildAuthUrl(validInputs)); + const techUrl = new URL(buildAuthUrl({ ...validInputs, appName: 'techupdates' })); + assert.notEqual( + noviUrl.searchParams.get('redirect_uri'), + techUrl.searchParams.get('redirect_uri'), + ); + assert.ok(techUrl.searchParams.get('redirect_uri').endsWith('/techupdates/oauth')); + }); + + test('returns null when clientId is missing', () => { + assert.equal(buildAuthUrl({ ...validInputs, clientId: undefined }), null); + assert.equal(buildAuthUrl({ ...validInputs, clientId: '' }), null); + }); + + test('returns null when template is missing', () => { + assert.equal(buildAuthUrl({ ...validInputs, template: undefined }), null); + assert.equal(buildAuthUrl({ ...validInputs, template: '' }), null); + }); +}); + +describe('cleanCompletedJobs', () => { + // Pin "now" to a fixed instant so the age arithmetic is deterministic. + const NOW = new Date('2026-07-01T12:00:00Z').getTime(); + const daysAgoIso = (n) => new Date(NOW - n * 24 * 60 * 60 * 1000).toISOString(); + + test('drops jobs older than the retention window', () => { + const jobs = { + completed: [ + { jobId: 'old', endTime: daysAgoIso(45) }, + { jobId: 'edge', endTime: daysAgoIso(31) }, + { jobId: 'recent', endTime: daysAgoIso(10) }, + ], + }; + const result = cleanCompletedJobs(jobs, 30, NOW); + assert.equal(result.removed, 2); + assert.deepEqual(result.jobs.completed.map(j => j.jobId), ['recent']); + }); + + test('keeps a job exactly at the cutoff (>=, not >)', () => { + const jobs = { + completed: [ + { jobId: 'on-cutoff', endTime: daysAgoIso(30) }, + ], + }; + const result = cleanCompletedJobs(jobs, 30, NOW); + assert.equal(result.removed, 0); + assert.equal(result.jobs.completed.length, 1); + }); + + test('falls back through endTime -> startTime -> created', () => { + const jobs = { + completed: [ + { jobId: 'byStart', startTime: daysAgoIso(60) }, + { jobId: 'byCreated', created: daysAgoIso(5) }, + ], + }; + const result = cleanCompletedJobs(jobs, 30, NOW); + assert.equal(result.removed, 1); + assert.equal(result.jobs.completed[0].jobId, 'byCreated'); + }); + + test('keeps jobs with no timestamp at all (safety default)', () => { + const jobs = { + completed: [ + { jobId: 'noDate' }, + { jobId: 'old', endTime: daysAgoIso(100) }, + ], + }; + const result = cleanCompletedJobs(jobs, 30, NOW); + assert.equal(result.removed, 1); + assert.deepEqual(result.jobs.completed.map(j => j.jobId), ['noDate']); + }); + + test('handles missing completed array without throwing', () => { + const jobs = {}; + const result = cleanCompletedJobs(jobs, 30, NOW); + assert.equal(result.removed, 0); + assert.equal(result.jobs, jobs); + }); + + test('defaults to COMPLETED_RETENTION_DAYS when retentionDays is omitted', () => { + const jobs = { + completed: [ + { jobId: 'stale', endTime: daysAgoIso(COMPLETED_RETENTION_DAYS + 5) }, + { jobId: 'ok', endTime: daysAgoIso(1) }, + ], + }; + const result = cleanCompletedJobs(jobs, undefined, NOW); + assert.equal(result.retentionDays, COMPLETED_RETENTION_DAYS); + assert.deepEqual(result.jobs.completed.map(j => j.jobId), ['ok']); + }); +}); + +describe('msToTime', () => { + test('formats sub-minute durations as "S.Ms"', () => { + assert.equal(msToTime(5300), '5.3s'); + }); + + test('formats sub-hour durations as "Mm S.Ms"', () => { + assert.equal(msToTime(90500), '1m 30.5s'); + }); + + test('formats hour-plus durations as "Hh Mm S.Ms"', () => { + // 1h 2m 3.4s + assert.equal(msToTime(3600000 + 2 * 60000 + 3400), '1h 2m 3.4s'); + }); + + test('handles zero', () => { + assert.equal(msToTime(0), '0.0s'); + }); +});