diff --git a/.configs/vitest.config.mjs b/.configs/vitest.config.mjs index fd25d2e..b0bedf6 100644 --- a/.configs/vitest.config.mjs +++ b/.configs/vitest.config.mjs @@ -8,6 +8,24 @@ const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); export default defineConfig({ root, + // The package-scoped dev condition that routes `@cldmv/holdmytask/main` to `src/` + // (see the `./main` export in package.json). Tests exercise and cover the SOURCE + // tree, so the resolver must add `holdmytask-dev`. This *replaces* vite's default + // conditions, so the usual ones are kept alongside it. `test.nodeOptions` below + // carries the same condition into forked test workers (for native imports of the + // package entry, e.g. CommonAliases importing index.mjs -> /main), so a bare local + // `npm test` resolves to src the same way CI does. Mirrors @cldmv/uuid. + resolve: { + conditions: ["holdmytask-dev", "module", "browser", "development|production"] + }, + ssr: { + // Vitest often routes node-environment resolution through the SSR pipeline. Keep + // `module` here alongside the non-SSR resolver so a dependency's `module`-keyed + // export resolves the same under Vitest's SSR pipeline as in a normal build. + resolve: { + conditions: ["holdmytask-dev", "module", "node", "development|production"] + } + }, test: { // Fleet-wide vitest test-file convention: `*.test.vitest.mjs`. include: ["tests/**/*.test.vitest.mjs"], @@ -15,6 +33,12 @@ export default defineConfig({ environment: "node", globals: true, testTimeout: 30000, + // Carry the dev condition into forked workers (native imports of the package + // entry, e.g. CommonAliases importing index.mjs -> /main). NODE_ENV is left + // alone: it does not select the conditional export (that's `--conditions`), and + // forcing a non-standard `NODE_ENV=holdmytask-dev` could confuse deps that key + // off the usual test/development/production values. + nodeOptions: ["--conditions=holdmytask-dev"], // "dot" keeps CI logs to one character per test file instead of a full // "RUN vX.Y.Z" + per-file pass/fail block for every file — vitest's // non-interactive fallback (no TTY to redraw) otherwise reprints that diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7505bf..25e77f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,14 +131,27 @@ on: required: false default: true -# Cancel superseded runs on feature branches; keep every master/main run as the -# permanent green record. Keyed on github.ref so push and pull_request events -# for the same branch share a group (the `if:` on the ci job already prevents -# non-fork PR sync from running, but the shared group guards against edge -# cases). +# Concurrency policy, by context: +# - FEATURE branches / feature PRs → cancel superseded runs (per-ref group + +# cancel-in-progress): a newer push makes the older run redundant. +# - RELEASE-relevant contexts → NEVER superseded. Pushes to the release base +# branch (derived: the CLDMV_RELEASE_BASE var → the repo's default branch), +# to next/hotfixes, and the next/hotfixes → base release PRs each get a +# UNIQUE group per run (run_id appended), so nothing cancels them. During +# the burst of pushes a release makes to next/hotfixes (the feature squash, +# the post-hotfix base→next sync merge, the bot's `chore: bump version`), +# every run completes and posts a GREEN check instead of the earlier one +# being cancelled into a red X on the release PR. A bare +# `cancel-in-progress: false` is NOT enough — GitHub still cancels the +# middle PENDING run when a newer one queues; a unique group avoids it. +# The base is NOT hardcoded to master/main — it derives the same way +# resolve-release-base does (CLDMV_RELEASE_BASE override → default_branch). +# next/hotfixes are the flow's fixed integration-branch names. github.head_ref +# is set only on pull_request (the release PR's head → next/hotfixes); +# github.ref carries the branch on push. concurrency: - group: ci-${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: ${{ github.ref != 'refs/heads/master' && github.ref != 'refs/heads/main' }} + group: ci-${{ github.workflow }}-${{ github.ref }}${{ (github.ref == format('refs/heads/{0}', vars.CLDMV_RELEASE_BASE != '' && vars.CLDMV_RELEASE_BASE || github.event.repository.default_branch) || github.ref == 'refs/heads/next' || github.ref == 'refs/heads/hotfixes' || github.head_ref == 'next' || github.head_ref == 'hotfixes') && format('-{0}', github.run_id) || '' }} + cancel-in-progress: true # Workflow-level: matches the broadest write surface the called # `workflow-ci.yml` reaches across its branches: diff --git a/devcheck.mjs b/devcheck.mjs index 9303010..aef5cf2 100644 --- a/devcheck.mjs +++ b/devcheck.mjs @@ -8,7 +8,7 @@ * @Last modified by: Nate Hyson (Shinrai@users.noreply.github.com) * @Last modified time: 2025-11-21 14:51:16 -08:00 (1763765476) * ----- - * @Copyright: Copyright (c) 2013-2025 Catalyzed Motivation Inc. All rights reserved. + * @Copyright: Copyright (c) 2013-2026 Catalyzed Motivation Inc. All rights reserved. */ import { existsSync } from "node:fs"; @@ -18,7 +18,6 @@ import path from "node:path"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const srcPath = path.join(__dirname, "src"); -// const distPath = path.join(__dirname, "dist"); // Detect if we're running in a CI environment const isCI = !!( @@ -32,29 +31,75 @@ const isCI = !!( process.env.TF_BUILD // Azure DevOps ); -if (existsSync(srcPath) && !isCI) { - // if (existsSync(srcPath) && !existsSync(distPath)) { - const nodeEnv = process.env.NODE_ENV?.toLowerCase(); - const hasNodeOptions = process.env.NODE_OPTIONS?.includes("--conditions=development"); +// Skip when installed as a dependency (a `node_modules` segment anywhere above this +// file - covers scoped `node_modules/@cldmv/holdmytask` and unscoped installs). The +// npm-published package ships neither `src/` nor this file, so this branch is already +// moot there; but a git/tarball install DOES include them, and without this guard +// devcheck would `process.exit(1)` inside a consumer's app. A "parent dir === +// node_modules" check would miss scoped packages (parent is the scope dir). +const isInstalledPackage = __dirname.split(path.sep).includes("node_modules"); - if (!nodeEnv || (!["dev", "development"].includes(nodeEnv) && !hasNodeOptions)) { +// Only meaningful in a source checkout. When `src/` is present the developer should be +// loading from it via the `holdmytask-dev` condition; if that condition isn't set they +// are silently running the built `dist/` copy instead, so warn - even after a build, +// since a built checkout has BOTH src/ and dist/ and the condition is the only thing +// that selects src/. +if (existsSync(srcPath) && !isCI && !isInstalledPackage) { + // The condition selects src/ (see the `./main` export in package.json). It can be + // supplied via NODE_OPTIONS (`NODE_OPTIONS=--conditions=holdmytask-dev`) OR directly + // on the node CLI (`node --conditions=holdmytask-dev` / `-C holdmytask-dev`), which + // lands in execArgv - this is how vitest passes it to workers - so scan both. Each + // `--conditions` occurrence is ONE literal condition value: Node does not split it on + // `,` or `|` (verified - `--conditions=holdmytask-dev,x` and + // `--conditions=holdmytask-dev|x` do NOT enable `holdmytask-dev`), and multiple + // conditions are passed as repeated flags. So collect each value whole and match + // EXACTLY - no substring, no splitting - so `not-holdmytask-dev`, `holdmytask-dev,x`, + // and `holdmytask-dev|production` all correctly fail to count. Namespaced (not the + // generic `development`) so a consuming app's own `--conditions=development` can't + // flip this package to a source tree it doesn't ship. NODE_ENV is deliberately NOT + // consulted: it does not affect which tree resolves. + const conditions = []; + const scan = (tokens) => { + for (let i = 0; i < tokens.length; i++) { + if (tokens[i] === "--conditions" || tokens[i] === "-C") { + // Space form (`--conditions x` / `-C x`): consume the following token as this + // flag's value and SKIP it, so a value that itself looks like a flag (e.g. the + // literal `--conditions=x`) isn't re-interpreted on the next iteration. + if (tokens[i + 1] !== undefined) { + conditions.push(tokens[i + 1]); + i++; + } + } else if (tokens[i].startsWith("--conditions=")) { + conditions.push(tokens[i].slice("--conditions=".length)); + } + // Note: `-C=x` is intentionally not handled - Node rejects it ("bad option"), + // so it can never appear in execArgv/NODE_OPTIONS. Valid forms are + // `--conditions=x`, `--conditions x`, and `-C x`. + } + }; + scan(process.execArgv); + scan((process.env.NODE_OPTIONS || "").split(/\s+/).filter(Boolean)); + const hasHoldMyTaskDev = conditions.includes("holdmytask-dev"); + + if (!hasHoldMyTaskDev) { console.error("❌ Development environment not properly configured!"); - console.error("📁 Source folder detected but NODE_ENV/NODE_OPTIONS not set for development."); + console.error("📁 Source folder detected but the 'holdmytask-dev' condition is not set,"); + console.error(" so imports resolve to dist/ by default (or fail if it isn't built) instead of src/."); console.error(""); - console.error("🔧 To fix this, run one of these commands:"); + console.error("🔧 To load from src/ for development, set the condition:"); console.error(" Windows (cmd):"); - console.error(" set NODE_ENV=development"); - console.error(" set NODE_OPTIONS=--conditions=development"); + console.error(" set NODE_OPTIONS=--conditions=holdmytask-dev"); console.error(""); console.error(" Windows (PowerShell):"); - console.error(" $env:NODE_ENV='development'"); - console.error(" $env:NODE_OPTIONS='--conditions=development'"); + console.error(" $env:NODE_OPTIONS='--conditions=holdmytask-dev'"); console.error(""); console.error(" Unix/Linux/macOS:"); - console.error(" export NODE_ENV=development"); - console.error(" export NODE_OPTIONS=--conditions=development"); + console.error(" export NODE_OPTIONS=--conditions=holdmytask-dev"); + console.error(""); + console.error(" ...or pass it directly: node --conditions=holdmytask-dev "); console.error(""); - console.error("💡 This ensures this module loads from src/ instead of dist/ for development."); + console.error("💡 'holdmytask-dev' is namespaced so it can't conflict with a consumer's"); + console.error(" own development conditions."); console.error("🚀 CI environments automatically skip this check."); process.exit(1); } diff --git a/index.mjs b/index.mjs index 8b26b48..878d647 100644 --- a/index.mjs +++ b/index.mjs @@ -11,7 +11,11 @@ * @Copyright: Copyright (c) 2013-2025 Catalyzed Motivation Inc. All rights reserved. */ -// Development environment check (must happen before holdmytask imports) +// Development environment check. NOTE: the static `import` of the core below is +// hoisted and evaluated before this IIFE body runs, so devcheck does NOT run before +// the core loads - it's a best-effort, fire-and-forget dev-time warning. (Running it +// strictly first would require a dynamic import + top-level await, which breaks the +// index.cjs bridge's synchronous `require` of this module - see PR #11 discussion.) (async () => { try { await import("./devcheck.mjs"); @@ -20,56 +24,51 @@ } })(); +import { HoldMyTask } from "@cldmv/holdmytask/main"; + /** * Creates a HoldMyTask instance for task queue management * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ -export default async function createHoldMyTask(options = {}) { - // Dynamic import after environment check - const mod = await import("@cldmv/holdmytask/main"); - const HoldMyTask = mod.HoldMyTask; +export async function createHoldMyTask(options = {}) { return new HoldMyTask(options); } /** * Create a task queue instance * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ export async function createQueue(options = {}) { - const mod = await import("@cldmv/holdmytask/main"); - const HoldMyTask = mod.HoldMyTask; return new HoldMyTask(options); } /** * Create a task manager instance * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ export async function createTaskManager(options = {}) { - const mod = await import("@cldmv/holdmytask/main"); - const HoldMyTask = mod.HoldMyTask; return new HoldMyTask(options); } /** * Create a task processor instance * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ export async function createTaskProcessor(options = {}) { - const mod = await import("@cldmv/holdmytask/main"); - const HoldMyTask = mod.HoldMyTask; return new HoldMyTask(options); } -// Named export aliases -export { createHoldMyTask as HoldMyTask }; -export { createQueue as queue }; -export { createQueue as Queue }; -export { createTaskManager as TaskManager }; -export { createQueue as TaskQueue }; -export { createQueue as QueueManager }; -export { createTaskProcessor as TaskProcessor }; +// HoldMyTask and its constructor aliases are the real class (see issue #3) - `new +// HoldMyTask()`, `new QueueManager()`, etc. all construct the same underlying type. +export { HoldMyTask }; +export default HoldMyTask; +export { HoldMyTask as queue }; +export { HoldMyTask as Queue }; +export { HoldMyTask as TaskManager }; +export { HoldMyTask as TaskQueue }; +export { HoldMyTask as QueueManager }; +export { HoldMyTask as TaskProcessor }; diff --git a/package-lock.json b/package-lock.json index 6753c0f..10d1dc9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@cldmv/holdmytask", - "version": "1.6.2", + "version": "2.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@cldmv/holdmytask", - "version": "1.6.2", + "version": "2.0.0", "license": "Apache-2.0", "devDependencies": { "@cldmv/vitest-runner": "^1.2.0", @@ -16,7 +16,7 @@ "@eslint/markdown": "^8.0.3", "@html-eslint/eslint-plugin": "^0.64.0", "@html-eslint/parser": "^0.64.0", - "@types/node": "^20.0.0", + "@types/node": "^26.1.2", "@vitest/coverage-v8": "^4.1.10", "@vitest/ui": "^4.1.10", "eslint": "^10.8.0", @@ -979,13 +979,13 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "20.19.24", - "resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.24.tgz", - "integrity": "sha512-FE5u0ezmi6y9OZEzlJfg37mqqf6ZDSF2V/NLjUyGrR9uTZ7Sb9F7bLNZ03S4XVUNRWGA7Ck4c1kK+YnuWjl+DA==", + "version": "26.1.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.1.2.tgz", + "integrity": "sha512-Vu4a5UFA9rIIFJ7rB/Vaafh9lrCQszopTCx6KjFboXTGQbPNasehVR5TEiithSDGyd1DEiUByggTZsg8jukeIg==", "dev": true, "license": "MIT", "dependencies": { - "undici-types": "~6.21.0" + "undici-types": "~8.3.0" } }, "node_modules/@types/unist": { @@ -3792,9 +3792,9 @@ } }, "node_modules/prettier": { - "version": "3.6.2", - "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.6.2.tgz", - "integrity": "sha512-I7AIg5boAr5R0FFtJ6rCfD+LFsWHp81dolrFD8S79U9tb8Az2nGrJncnMSnys+bpQJfRUzqs9hnA81OAA3hCuQ==", + "version": "3.9.6", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.9.6.tgz", + "integrity": "sha512-OpN0zzVdiaiAhxpuuj5efpIS4sY9j7bY6uR5mnj5yPzGkdkjNKSJeUThPb60Jw29QuAZgA4o+/iB49kFiaBX6g==", "dev": true, "license": "MIT", "bin": { @@ -4057,9 +4057,9 @@ } }, "node_modules/undici-types": { - "version": "6.21.0", - "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.21.0.tgz", - "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==", + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz", + "integrity": "sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ==", "dev": true, "license": "MIT" }, diff --git a/package.json b/package.json index 8a8e78c..d1de1b8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@cldmv/holdmytask", - "version": "1.6.2", + "version": "2.0.0", "description": "A tiny task queue that waits until your task is ready", "main": "./index.cjs", "module": "./index.mjs", @@ -11,12 +11,8 @@ "import": "./index.mjs", "require": "./index.cjs" }, - "./devcheck": { - "types": "./types/devcheck.d.mts", - "import": "./devcheck.mjs" - }, "./main": { - "development": { + "holdmytask-dev": { "types": "./types/src/hold-my-task.d.mts", "import": "./src/hold-my-task.mjs" }, @@ -91,7 +87,7 @@ "@eslint/markdown": "^8.0.3", "@html-eslint/eslint-plugin": "^0.64.0", "@html-eslint/parser": "^0.64.0", - "@types/node": "^20.0.0", + "@types/node": "^26.1.2", "@vitest/coverage-v8": "^4.1.10", "@vitest/ui": "^4.1.10", "eslint": "^10.8.0", diff --git a/src/hold-my-task.mjs b/src/hold-my-task.mjs index 9cdb198..a428686 100644 --- a/src/hold-my-task.mjs +++ b/src/hold-my-task.mjs @@ -1622,6 +1622,16 @@ export class HoldMyTask extends EventEmitter { nextTime = Math.min(nextTime, this.nextAvailableTime); } + // A task already sitting in the ready heap needs an imminent recheck if nothing + // else would schedule one - otherwise nextTime falls through to Infinity and the + // 24.8-day fallback below strands it. Only force this when nextTime would + // otherwise stay Infinity (the stranded-task case); an active future delay + // already set nextTime above and should keep its single efficient timeout + // rather than degrade into a tick-interval poll loop until it expires. + if (nextTime === Infinity && this.readyHeap.size() > 0) { + nextTime = now; + } + // If next event is imminent or past, run immediately if (nextTime <= now + this.options.tick) { this.intervalId = setInterval(() => this.schedulerTick(), this.options.tick); diff --git a/tests/BypassDelayCompletionTiming.test.vitest.mjs b/tests/BypassDelayCompletionTiming.test.vitest.mjs new file mode 100644 index 0000000..9936455 --- /dev/null +++ b/tests/BypassDelayCompletionTiming.test.vitest.mjs @@ -0,0 +1,160 @@ +import { test, expect, describe, vi } from "vitest"; +import { HoldMyTask } from "../src/hold-my-task.mjs"; + +// Isolated from HoldMyTask.test.vitest.mjs: this test hangs until the 30s Vitest +// timeout on the `lts/*` CI matrix job specifically (never on the explicit 20/22/24 +// jobs running the identical Node binary), and never reproduces locally even under +// CPU throttling. Splitting it into its own file removes it from the tail end of a +// 72-test file where ~30 prior HoldMyTask instances are never destroy()'d, to check +// whether accumulated per-instance timer/state leakage from earlier tests in the same +// file is a factor. +describe.each([ + { smartScheduling: true, mode: "Smart Scheduling" }, + { smartScheduling: false, mode: "Traditional Polling" } +])("HoldMyTask with $mode", ({ smartScheduling }) => { + test("bypassed task still applies its own completion delay", async () => { + const q = new HoldMyTask({ + concurrency: 1, + delays: { 1: 200, 2: 400 }, + smartScheduling + }); + const results = []; + const timestamps = []; + + // Task 1: Priority 1 (200ms delay) - will complete first + q.enqueue( + () => { + timestamps.push(Date.now()); + return "task1"; + }, + (err, result) => results.push(result), + { priority: 1 } + ); + + // Task 2: Same priority but bypasses the delay from task1 + q.enqueue( + () => { + timestamps.push(Date.now()); + return "task2"; + }, + (err, result) => results.push(result), + { priority: 1, bypassDelay: true } + ); + + // Task 3: Should wait for whatever delay task2 creates (task2 has no specific delay config, so uses priority 1 = 200ms) + q.enqueue( + () => { + timestamps.push(Date.now()); + return "task3"; + }, + (err, result) => results.push(result), + { priority: 1 } + ); + + try { + // Race the drain wait against a short timeout: if drain never fires (the exact + // deadlock this file exists to catch), fail fast instead of hanging to vitest's + // 30s global timeout - and keep the wait inside the try so the finally still + // runs q.destroy() and doesn't leak timers into later tests. + await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("drain not emitted within 5000ms")), 5000); + q.once("drain", () => { + clearTimeout(timer); + resolve(); + }); + }); + + expect(results).toEqual(["task1", "task2", "task3"]); // Normal priority order, task2 bypasses task1's delay + + // Task2 bypasses task1's delay (should start immediately after task1) + const task1ToTask2Gap = timestamps[1] - timestamps[0]; + expect(task1ToTask2Gap).toBeLessThan(100); + + // Task3 waits for task2's completion delay (200ms since task2 is priority 1) + const task2ToTask3Gap = timestamps[2] - timestamps[1]; + expect(task2ToTask3Gap).toBeGreaterThan(150); + } finally { + q.destroy(); + } + }); +}); + +// Regression test for the scheduler deadlock fixed alongside the test above: +// _scheduleNextTick() only derived its next wake time from pendingHeap and +// nextAvailableTime. Called at the exact moment nextAvailableTime had just +// expired (a narrow race against schedulerTick's own timing), with pendingHeap +// empty, nextTime fell through to Infinity - arming a ~24.8-day setTimeout and +// stranding any task already sitting in readyHeap forever. Traditional Polling +// only: Smart Scheduling uses a different scheduleSmartTimeout()/runScheduler() +// path unaffected by this bug. Uses the constructor's injectable `now` option to +// force the exact race deterministically instead of racing real wall-clock time. +test("a readyHeap task is not stranded when its delay has just expired (regression)", async () => { + let fakeNow = Date.now(); + const q = new HoldMyTask({ + concurrency: 1, + delays: { 1: 50 }, + smartScheduling: false, + now: () => fakeNow + }); + const results = []; + + q.enqueue( + () => "task1", + (err, r) => results.push(r), + { priority: 1 } + ); + q.enqueue( + () => "task2", + (err, r) => results.push(r), + { priority: 1 } + ); + + try { + // Wait (bounded) until task1 has actually run + completed and task2 has moved + // into readyHeap, blocked by task1's post-completion delay - polling the + // observable state instead of a fixed sleep, which is nondeterministic on slow/ + // contended CI. The queue's delay bookkeeping uses the injected now(), not real + // time, so task2 stays parked in readyHeap. Kept inside the try so the finally + // still runs q.destroy() if this ever times out. + const deadline = Date.now() + 2000; + while (!(results.includes("task1") && q.readyHeap.size() === 1 && q.pendingHeap.size() === 0)) { + if (Date.now() > deadline) { + throw new Error("task1 did not complete / task2 did not reach readyHeap within 2000ms"); + } + await new Promise((resolve) => setTimeout(resolve, 5)); + } + + // task2 is now sitting in readyHeap, blocked by task1's post-completion delay. + expect(q.readyHeap.size()).toBe(1); + expect(q.pendingHeap.size()).toBe(0); + expect(q.nextAvailableTime).toBeGreaterThan(0); + + // Simulate wall-clock time crossing nextAvailableTime right before the + // scheduler gets a chance to recheck it - the exact race window that caused + // the deadlock. + fakeNow = q.nextAvailableTime + 1; + + const setTimeoutSpy = vi.spyOn(global, "setTimeout"); + let armedCalls; + try { + q._scheduleNextTick(); + } finally { + // Capture the recorded calls BEFORE restoring, and restore in finally so the + // global spy never leaks into later tests even if _scheduleNextTick throws. + armedCalls = setTimeoutSpy.mock.calls.slice(); + setTimeoutSpy.mockRestore(); + } + + if (armedCalls.length > 0) { + const armedDelay = armedCalls.at(-1)[1]; + // Before the fix this was ~2147483647 (the 24.8-day fallback), stranding + // task2 forever. After the fix it must be an imminent recheck. + expect(armedDelay).toBeLessThan(1000); + } else { + // Took the interval branch instead - also an imminent recheck, also fine. + expect(q.intervalId).toBeTruthy(); + } + } finally { + q.destroy(); + } +}); diff --git a/tests/CommonAliases.test.vitest.mjs b/tests/CommonAliases.test.vitest.mjs index b853c13..f7f560f 100644 --- a/tests/CommonAliases.test.vitest.mjs +++ b/tests/CommonAliases.test.vitest.mjs @@ -8,21 +8,10 @@ * @Copyright: Copyright (c) 2013-2025 Catalyzed Motivation Inc. All rights reserved. */ -// Whole file skipped: every describe block below assumes HoldMyTask / -// Queue / TaskManager / TaskQueue / QueueManager / TaskProcessor (as -// imported from "../index.mjs") are constructors ("new HoldMyTask()" -// etc.), but index.mjs actually exports async factory functions -// (createHoldMyTask() and friends) under those names. This is a -// pre-existing mismatch between the test suite and the real runtime -// exports, unrelated to the v4 CI/vitest-runner onboarding that first -// wired these tests into CI — see -// https://github.com/CLDMV/holdmytask/issues/3. Un-skip once that's -// resolved (either the aliases become real constructors, or these -// tests are rewritten to call the factories instead of `new`-ing them). import { test, expect, describe } from "vitest"; import { HoldMyTask, Queue, TaskManager, TaskQueue, QueueManager, TaskProcessor } from "../index.mjs"; -describe.skip("Common Queue System Aliases", () => { +describe("Common Queue System Aliases", () => { test("should export HoldMyTask as the main class", () => { expect(HoldMyTask).toBeDefined(); expect(typeof HoldMyTask).toBe("function"); @@ -340,7 +329,7 @@ describe.skip("Common Queue System Aliases", () => { }); }); -describe.skip("Primary Method Names", () => { +describe("Primary Method Names", () => { test("has() should work as primary method", () => { const queue = new HoldMyTask(); const customId = "primary-has-test"; @@ -393,7 +382,7 @@ describe.skip("Primary Method Names", () => { }); }); -describe.skip("Method Alias Compatibility", () => { +describe("Method Alias Compatibility", () => { test("hasTask() should work as alias for has()", () => { const queue = new HoldMyTask(); const customId = "hasTask-alias-test"; @@ -456,7 +445,7 @@ describe.skip("Method Alias Compatibility", () => { }); }); -describe.skip("Enqueue Method Aliases", () => { +describe("Enqueue Method Aliases", () => { test("schedule() should work as alias for enqueue()", () => { const queue = new HoldMyTask(); let executed = false; @@ -538,7 +527,7 @@ describe.skip("Enqueue Method Aliases", () => { }); }); -describe.skip("Import Aliases", () => { +describe("Import Aliases", () => { test("queue alias should work", async () => { const { queue } = await import("../index.mjs"); const instance = new queue(); @@ -606,7 +595,7 @@ describe.skip("Import Aliases", () => { }); }); -describe.skip("Control Method Aliases", () => { +describe("Control Method Aliases", () => { test("shutdown() should work as alias for destroy()", async () => { const queue = new HoldMyTask(); diff --git a/tests/DevCheck.test.vitest.mjs b/tests/DevCheck.test.vitest.mjs new file mode 100644 index 0000000..e086a0d --- /dev/null +++ b/tests/DevCheck.test.vitest.mjs @@ -0,0 +1,159 @@ +/** + * @Project: @cldmv/holdmytask + * @Filename: /tests/DevCheck.test.vitest.mjs + * @Date: 2026-08-08T00:00:00-08:00 (1786233600) + * @Author: Nate Hyson + * @Email: + * ----- + * @Last modified by: Nate Hyson (Shinrai@users.noreply.github.com) + * @Last modified time: 2026-08-08T00:00:00-08:00 (1786233600) + * ----- + * @Copyright: Copyright (c) 2013-2026 Catalyzed Motivation Inc. All rights reserved. + */ + +import { test, expect, describe, beforeAll, afterAll } from "vitest"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, copyFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +// devcheck.mjs resolves `src/` relative to its own file location and reads process.env +// / process.execArgv, so each case runs a COPY of it in a purpose-built fixture +// directory with a from-scratch env (only PATH), preventing the real CI environment +// this suite runs in from leaking `CI`/`GITHUB_ACTIONS`/`NODE_OPTIONS` into the +// subprocess. +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const devcheckSrc = path.join(repoRoot, "devcheck.mjs"); + +let tmpRoot; +let counter = 0; + +beforeAll(() => { + tmpRoot = mkdtempSync(path.join(tmpdir(), "holdmytask-devcheck-")); +}); + +afterAll(() => { + if (tmpRoot) rmSync(tmpRoot, { recursive: true, force: true }); +}); + +// Materialize a fixture dir with a copy of devcheck.mjs plus optional src/ and dist/, +// optionally nested under node_modules// to simulate an installed package. +function makeFixture({ src = true, dist = false, installed = false } = {}) { + const base = path.join(tmpRoot, `f${counter++}`); + const pkgDir = installed ? path.join(base, "node_modules", "@cldmv", "holdmytask") : path.join(base, "holdmytask"); + mkdirSync(pkgDir, { recursive: true }); + if (src) mkdirSync(path.join(pkgDir, "src"), { recursive: true }); + if (dist) mkdirSync(path.join(pkgDir, "dist"), { recursive: true }); + copyFileSync(devcheckSrc, path.join(pkgDir, "devcheck.mjs")); + return path.join(pkgDir, "devcheck.mjs"); +} + +// nodeArgs are passed on the node CLI (i.e. become process.execArgv); env is a +// from-scratch environment (only PATH plus whatever is given). +function runDevcheck(fixtureOpts, { env = {}, nodeArgs = [] } = {}) { + const devcheck = makeFixture(fixtureOpts); + const result = spawnSync(process.execPath, [...nodeArgs, devcheck], { + env: { PATH: process.env.PATH, ...env }, + encoding: "utf8" + }); + return { status: result.status, stderr: result.stderr || "" }; +} + +describe("devcheck", () => { + test("nags in a source checkout when the holdmytask-dev condition is not set", () => { + const { status, stderr } = runDevcheck({ src: true }); + expect(status).toBe(1); + expect(stderr).toContain("Development environment not properly configured"); + expect(stderr).toContain("--conditions=holdmytask-dev"); + }); + + test("stays silent when the condition is set via NODE_OPTIONS", () => { + const { status, stderr } = runDevcheck({ src: true }, { env: { NODE_OPTIONS: "--conditions=holdmytask-dev" } }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); + + test("stays silent when the condition is passed on the node CLI (execArgv, = form)", () => { + // vitest passes --conditions to workers this way, so devcheck must detect it here too. + const { status, stderr } = runDevcheck({ src: true }, { nodeArgs: ["--conditions=holdmytask-dev"] }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); + + test("stays silent when the condition is passed space-separated (--conditions holdmytask-dev)", () => { + const { status, stderr } = runDevcheck({ src: true }, { nodeArgs: ["--conditions", "holdmytask-dev"] }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); + + test("stays silent when holdmytask-dev is one of several repeated --conditions flags", () => { + const { status } = runDevcheck({ src: true }, { nodeArgs: ["--conditions=foo", "--conditions=holdmytask-dev"] }); + expect(status).toBe(0); + }); + + test("stays silent via the -C short flag (Node's alias for --conditions)", () => { + // Node accepts `-C ` (space form) but rejects `-C=`, so only the + // space form is a real input to detect. + const { status, stderr } = runDevcheck({ src: true }, { nodeArgs: ["-C", "holdmytask-dev"] }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); + + test("NODE_ENV=development alone does NOT silence it (only the condition selects src/)", () => { + // Keying off NODE_ENV would be a false negative: dev env set but no condition means + // the package is still resolving to dist/, which is exactly what should be flagged. + const { status } = runDevcheck({ src: true }, { env: { NODE_ENV: "development" } }); + expect(status).toBe(1); + }); + + test("STILL nags when dist/ has been built but the condition is not set", () => { + // A build must NOT silence the check: with src/ present the developer should be on + // src/ via the condition, not the stale dist/. + const { status } = runDevcheck({ src: true, dist: true }); + expect(status).toBe(1); + }); + + test("does NOT accept a generic development condition (namespacing)", () => { + const { status } = runDevcheck({ src: true }, { nodeArgs: ["--conditions=development"] }); + expect(status).toBe(1); + }); + + test("does NOT match a condition that merely contains 'holdmytask-dev' as a substring", () => { + // Exact-value match, not substring: --conditions=not-holdmytask-dev must NOT silence it. + const { status } = runDevcheck({ src: true }, { nodeArgs: ["--conditions=not-holdmytask-dev"] }); + expect(status).toBe(1); + }); + + test("does NOT treat a comma-joined value as separate conditions", () => { + // Node does not split --conditions on `,`: `foo,holdmytask-dev,bar` is one literal + // condition, so holdmytask-dev is NOT enabled and devcheck must still nag. + const { status } = runDevcheck({ src: true }, { nodeArgs: ["--conditions=foo,holdmytask-dev,bar"] }); + expect(status).toBe(1); + }); + + test("does NOT treat a pipe-joined value as separate conditions", () => { + // Likewise Node does not split on `|` (it's a valid condition character, e.g. + // Vite's `development|production`): `holdmytask-dev|production` does not enable it. + const { status } = runDevcheck({ src: true }, { nodeArgs: ["--conditions=holdmytask-dev|production"] }); + expect(status).toBe(1); + }); + + test("skips in CI", () => { + const { status, stderr } = runDevcheck({ src: true }, { env: { CI: "true" } }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); + + test("skips when installed as a scoped dependency (node_modules/@cldmv/holdmytask)", () => { + const { status, stderr } = runDevcheck({ src: true, installed: true }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); + + test("does nothing when there is no src/ (published dist-only layout)", () => { + const { status, stderr } = runDevcheck({ src: false, dist: true }); + expect(status).toBe(0); + expect(stderr).toBe(""); + }); +}); diff --git a/tests/HoldMyTask.test.vitest.mjs b/tests/HoldMyTask.test.vitest.mjs index 82ea457..1ed7331 100644 --- a/tests/HoldMyTask.test.vitest.mjs +++ b/tests/HoldMyTask.test.vitest.mjs @@ -803,58 +803,6 @@ describe.each([ // Test passed if no timeout occurred (task2 started immediately) }); - test("bypassed task still applies its own completion delay", async () => { - const q = new HoldMyTask({ - concurrency: 1, - delays: { 1: 200, 2: 400 }, - smartScheduling - }); - const results = []; - const timestamps = []; - - // Task 1: Priority 1 (200ms delay) - will complete first - q.enqueue( - () => { - timestamps.push(Date.now()); - return "task1"; - }, - (err, result) => results.push(result), - { priority: 1 } - ); - - // Task 2: Same priority but bypasses the delay from task1, priority 2 means higher priority but has delay - q.enqueue( - () => { - timestamps.push(Date.now()); - return "task2"; - }, - (err, result) => results.push(result), - { priority: 1, bypassDelay: true } - ); - - // Task 3: Should wait for whatever delay task2 creates (task2 has no specific delay config, so uses priority 1 = 200ms) - q.enqueue( - () => { - timestamps.push(Date.now()); - return "task3"; - }, - (err, result) => results.push(result), - { priority: 1 } - ); - - await new Promise((resolve) => q.on("drain", resolve)); - - expect(results).toEqual(["task1", "task2", "task3"]); // Normal priority order, task2 bypasses task1's delay - - // Task2 bypasses task1's delay (should start immediately after task1) - const task1ToTask2Gap = timestamps[1] - timestamps[0]; - expect(task1ToTask2Gap).toBeLessThan(100); - - // Task3 waits for task2's completion delay (200ms since task2 is priority 1) - const task2ToTask3Gap = timestamps[2] - timestamps[1]; - expect(task2ToTask3Gap).toBeGreaterThan(150); - }); - test("handles maxQueue: -1 as unlimited queue", async () => { const q = new HoldMyTask({ smartScheduling, maxQueue: -1 }); diff --git a/types/examples/priority-stress-test.d.mts b/types/examples/priority-stress-test.d.mts index 16a1388..ae7f36d 100644 --- a/types/examples/priority-stress-test.d.mts +++ b/types/examples/priority-stress-test.d.mts @@ -60,7 +60,7 @@ declare class PriorityVolumeController { declare function runPriorityStressTests(): Promise<{ scenario: string; totalDuration: number; - accurateCommands: number; + accurateCommands: any; totalCommands: number; accuracyRate: number; finalVolume: number; diff --git a/types/index.d.mts b/types/index.d.mts index 911b5fe..af165b3 100644 --- a/types/index.d.mts +++ b/types/index.d.mts @@ -10,35 +10,37 @@ * ----- * @Copyright: Copyright (c) 2013-2025 Catalyzed Motivation Inc. All rights reserved. */ +import { HoldMyTask } from "@cldmv/holdmytask/main"; /** * Creates a HoldMyTask instance for task queue management * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ -export default function createHoldMyTask(options?: object): Promise; +export declare function createHoldMyTask(options?: object): Promise; /** * Create a task queue instance * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ -export declare function createQueue(options?: object): Promise; +export declare function createQueue(options?: object): Promise; /** * Create a task manager instance * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ -export declare function createTaskManager(options?: object): Promise; +export declare function createTaskManager(options?: object): Promise; /** * Create a task processor instance * @param {object} [options={}] - Configuration options - * @returns {Promise} HoldMyTask instance + * @returns {Promise} HoldMyTask instance */ -export declare function createTaskProcessor(options?: object): Promise; -export { createHoldMyTask as HoldMyTask }; -export { createQueue as queue }; -export { createQueue as Queue }; -export { createTaskManager as TaskManager }; -export { createQueue as TaskQueue }; -export { createQueue as QueueManager }; -export { createTaskProcessor as TaskProcessor }; +export declare function createTaskProcessor(options?: object): Promise; +export { HoldMyTask }; +export default HoldMyTask; +export { HoldMyTask as queue }; +export { HoldMyTask as Queue }; +export { HoldMyTask as TaskManager }; +export { HoldMyTask as TaskQueue }; +export { HoldMyTask as QueueManager }; +export { HoldMyTask as TaskProcessor }; //# sourceMappingURL=index.d.mts.map \ No newline at end of file diff --git a/types/index.d.mts.map b/types/index.d.mts.map index 72a5e49..9a1ce9d 100644 --- a/types/index.d.mts.map +++ b/types/index.d.mts.map @@ -1 +1 @@ -{"version":3,"file":"index.d.mts","sourceRoot":"","sources":["../index.mjs"],"names":[],"mappings":"AAAA;;;;;;;;;;;GAWG;AAWH;;;;GAIG;AACH,wBAA8B,gBAAgB,CAAC,OAAO,AAHnD,CACA,EADQ,MAGgD,GAF9C,OAAO,CAAC,MAAM,CAAC,CAO3B;AAED;;;;GAIG;AACH,wBAAsB,WAAW,CAAC,OAAO,AAHtC,CACA,EADQ,MAGmC,GAFjC,OAAO,CAAC,MAAM,CAAC,CAM3B;AAED;;;;GAIG;AACH,wBAAsB,iBAAiB,CAAC,OAAO,AAH5C,CACA,EADQ,MAGyC,GAFvC,OAAO,CAAC,MAAM,CAAC,CAM3B;AAED;;;;GAIG;AACH,wBAAsB,mBAAmB,CAAC,OAAO,AAH9C,CACA,EADQ,MAG2C,GAFzC,OAAO,CAAC,MAAM,CAAC,CAM3B;AAGD,OAAO,EAAE,gBAAgB,IAAI,UAAU,EAAE,CAAC;AAC1C,OAAO,EAAE,WAAW,IAAI,KAAK,EAAE,CAAC;AAChC,OAAO,EAAE,WAAW,IAAI,KAAK,EAAE,CAAC;AAChC,OAAO,EAAE,iBAAiB,IAAI,WAAW,EAAE,CAAC;AAC5C,OAAO,EAAE,WAAW,IAAI,SAAS,EAAE,CAAC;AACpC,OAAO,EAAE,WAAW,IAAI,YAAY,EAAE,CAAC;AACvC,OAAO,EAAE,mBAAmB,IAAI,aAAa,EAAE,CAAC"} \ No newline at end of file +{"version":3,"file":"index.d.mts","sourceRoot":"","sources":["../index.mjs"],"names":[],"mappings":"AAAA;;;;;;;;;;;GAWG;AAeH,OAAO,EAAE,UAAU,EAAE,MAAM,wBAAwB,CAAC;AAEpD;;;;GAIG;AACH,wBAAsB,gBAAgB,CAAC,OAAO,AAH3C,CACA,EADQ,MAGwC,GAFtC,OAAO,CAAC,UAAU,CAAC,CAI/B;AAED;;;;GAIG;AACH,wBAAsB,WAAW,CAAC,OAAO,AAHtC,CACA,EADQ,MAGmC,GAFjC,OAAO,CAAC,UAAU,CAAC,CAI/B;AAED;;;;GAIG;AACH,wBAAsB,iBAAiB,CAAC,OAAO,AAH5C,CACA,EADQ,MAGyC,GAFvC,OAAO,CAAC,UAAU,CAAC,CAI/B;AAED;;;;GAIG;AACH,wBAAsB,mBAAmB,CAAC,OAAO,AAH9C,CACA,EADQ,MAG2C,GAFzC,OAAO,CAAC,UAAU,CAAC,CAI/B;AAID,OAAO,EAAE,UAAU,EAAE,CAAC;eACP,UAAU;AACzB,OAAO,EAAE,UAAU,IAAI,KAAK,EAAE,CAAC;AAC/B,OAAO,EAAE,UAAU,IAAI,KAAK,EAAE,CAAC;AAC/B,OAAO,EAAE,UAAU,IAAI,WAAW,EAAE,CAAC;AACrC,OAAO,EAAE,UAAU,IAAI,SAAS,EAAE,CAAC;AACnC,OAAO,EAAE,UAAU,IAAI,YAAY,EAAE,CAAC;AACtC,OAAO,EAAE,UAAU,IAAI,aAAa,EAAE,CAAC"} \ No newline at end of file