From 6234961991e0464d6249ca1c793f781d82a7159a Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 6 Jul 2026 16:04:40 +0200 Subject: [PATCH 1/8] refactor(project): Validate dependency caches in background after each build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cold cache lookup for a build-as-side-effect dependency currently waits for the first HTTP request against it. Move that lookup between build cycles so the server reports IDLE or STALE while it happens. New surface: - `ProjectBuildCache.validateCache(dependencyReader)` — split out of `prepareProjectBuildAndValidateCache`, which now only performs build-prep side effects and delegates. - `ProjectBuildContext.validateCache()` — logs perf timing, propagates changed paths to dependents. Cache.Force still throws on stale caches. - `ProjectBuilder.validateCaches({projects, signal}, callback)` — mirrors `#build`: guards `#buildIsRunning`, walks contexts dependency-first via `getRequiredProjectContexts` so cold contexts (and their transitive build-time dependencies' source indices) initialize inside the pass. A `willValidate` hook fires before each project so the caller can claim it. State machine: - `ProjectBuildStatus` gains a VALIDATING state; `setReader` accepts both BUILDING and VALIDATING as legitimate prior states. - `SERVER_STATES.VALIDATING` is distinct from BUILDING and STALE. After a cycle drains, BuildServer transitions BUILDING → VALIDATING when any project is still INITIAL, then to IDLE once the pass confirms every cache is fresh, or to STALE when the pass surfaces a stale one. Previously the post-build transition pinned to STALE, so a fully-cached graph never returned to IDLE on its own. - VALIDATING → STALE mirrors IDLE → STALE, so a source change during a pass surfaces immediately. `getReaderForProject` skips enqueueing a build while VALIDATING (the reader request joins the pass); on settle, any non-fresh project with pending readers gets a real build queued so callers don't hang. Error handling matches the build path in two directions: - A non-abort failure inside the pass emits `error` and transitions to SERVER_STATES.ERROR; the post-pass finally skips the IDLE/STALE transition when the run ended in ERROR. - A failed build cycle short-circuits the cycle-end transition. Before, `#build` resolved normally after setting ERROR and the cycle-end `.then()` scheduled a validation pass over every INITIAL project — in Cache.Force mode with no cache, that threw `Cache is in "Force" mode but no cache found` on the first dependency it visited, on top of the build error that just fired. BuildServer preempts the in-flight pass before each build cycle and on `destroy()`, composes every targeted project's per-project AbortSignal into the pass signal, and releases VALIDATING on every targeted project in a finally so none are stranded after an abort. The `Serve` logger gains `validating(projects)`, emitting `serve-validating`. The InteractiveConsole writer renders STATES.VALIDATING with a cyan spinner and keeps the tick loop running. --- packages/logger/lib/loggers/Serve.js | 16 + .../logger/lib/writers/InteractiveConsole.js | 15 +- .../lib/writers/interactiveConsole/render.js | 13 + .../writers/interactiveConsole/state/build.js | 4 + packages/logger/test/lib/loggers/Serve.js | 32 ++ .../test/lib/writers/InteractiveConsole.js | 53 ++ .../lib/writers/interactiveConsole/render.js | 25 + .../writers/interactiveConsole/state/build.js | 1 + packages/project/lib/build/BuildServer.js | 289 ++++++++++- packages/project/lib/build/ProjectBuilder.js | 149 +++++- .../lib/build/cache/ProjectBuildCache.js | 25 +- .../lib/build/helpers/ProjectBuildContext.js | 44 +- packages/project/lib/build/helpers/abort.js | 18 + .../project/test/lib/build/BuildServer.js | 474 ++++++++++++++++++ .../project/test/lib/build/ProjectBuilder.js | 153 ++++++ .../test/lib/build/cache/ProjectBuildCache.js | 90 +++- 16 files changed, 1372 insertions(+), 29 deletions(-) create mode 100644 packages/project/lib/build/helpers/abort.js diff --git a/packages/logger/lib/loggers/Serve.js b/packages/logger/lib/loggers/Serve.js index b9d27f8d63d..9f5c8f3b6de 100644 --- a/packages/logger/lib/loggers/Serve.js +++ b/packages/logger/lib/loggers/Serve.js @@ -45,6 +45,22 @@ class Serve extends Logger { } } + validating(validatingProjects) { + if (!validatingProjects || !Array.isArray(validatingProjects)) { + throw new Error( + "loggers/Serve#validating: Missing or incorrect validatingProjects parameter"); + } + const level = "info"; + const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { + level, + status: "serve-validating", + validatingProjects, + }); + if (!hasListeners) { + this._log(level, `Validating caches for: ${validatingProjects.join(", ")}`); + } + } + building() { const level = "info"; const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { diff --git a/packages/logger/lib/writers/InteractiveConsole.js b/packages/logger/lib/writers/InteractiveConsole.js index b0141835204..04b37db57f3 100644 --- a/packages/logger/lib/writers/InteractiveConsole.js +++ b/packages/logger/lib/writers/InteractiveConsole.js @@ -15,7 +15,7 @@ import { renderHeaderRegion, renderProjectRegion, renderServerRegion, renderBuildRegion, } from "./interactiveConsole/render.js"; -// Spinner tick interval while in `building` state. +// Spinner tick interval while in `building` or `validating` state. const BUILDING_TICK_MS = 120; // Decode the tail of `stream.write(chunk[, encoding][, callback])`. `encoding` @@ -341,6 +341,11 @@ class InteractiveConsole { } #handleServeStatus(evt) { + // validatingProjects is a VALIDATING-only payload; clear it on any other + // transition so a stale list doesn't linger in the banner state. + if (evt.status !== "serve-validating") { + this.#buildState.validatingProjects = []; + } switch (evt.status) { case "serve-ready": this.#transitionTo(STATES.READY); @@ -353,6 +358,11 @@ class InteractiveConsole { beginBuild(this.#buildState, this.#buildState.projectOrder); this.#transitionTo(STATES.BUILDING); break; + case "serve-validating": + this.#buildState.validatingProjects = evt.validatingProjects || []; + this.#buildState.spinFrame = 0; + this.#transitionTo(STATES.VALIDATING); + break; case "serve-build-done": this.#buildState.lastBuildHrtime = Array.isArray(evt.hrtime) ? evt.hrtime : null; this.#transitionTo(STATES.READY); @@ -393,7 +403,8 @@ class InteractiveConsole { if (this.#stopped) { return; } - if (this.#buildState.state !== STATES.BUILDING) { + if (this.#buildState.state !== STATES.BUILDING && + this.#buildState.state !== STATES.VALIDATING) { return; } this.#tickTimer = setInterval(() => { diff --git a/packages/logger/lib/writers/interactiveConsole/render.js b/packages/logger/lib/writers/interactiveConsole/render.js index 52501cac550..615aadbf0d0 100644 --- a/packages/logger/lib/writers/interactiveConsole/render.js +++ b/packages/logger/lib/writers/interactiveConsole/render.js @@ -150,6 +150,19 @@ function renderStatusLine(state) { case STATES.STALE: return `${label}${chalk.yellow(figures.circle)} ${chalk.yellow(pad("stale"))} ` + `${chalk.dim("· files changed, rebuild on next request")}`; + case STATES.VALIDATING: { + const frame = SPINNER_FRAMES[state.spinFrame % SPINNER_FRAMES.length]; + const parts = [ + `${chalk.cyan(frame)} ${chalk.cyan(pad("validating cache"))}`, + chalk.dim("·"), + chalk.dim("checking dependency caches"), + ]; + const count = state.validatingProjects.length; + if (count > 0) { + parts.push(chalk.dim("·"), chalk.dim(`${count} project${count === 1 ? "" : "s"}`)); + } + return `${label}${parts.join(" ")}`; + } case STATES.BUILDING: { const frame = SPINNER_FRAMES[state.spinFrame % SPINNER_FRAMES.length]; const parts = [ diff --git a/packages/logger/lib/writers/interactiveConsole/state/build.js b/packages/logger/lib/writers/interactiveConsole/state/build.js index e62fd6f4aad..a620fb88145 100644 --- a/packages/logger/lib/writers/interactiveConsole/state/build.js +++ b/packages/logger/lib/writers/interactiveConsole/state/build.js @@ -7,6 +7,7 @@ export const STATES = Object.freeze({ READY: "ready", STALE: "stale", BUILDING: "building", + VALIDATING: "validating", ERROR: "error", }); @@ -22,6 +23,9 @@ export function createBuildState() { // Names of projects collected via `serve-stale` payloads — used to label // the stale state if/when the renderer wants to. changedProjects: [], + // Names of projects collected via `serve-validating` payloads — used to + // label the validating state if/when the renderer wants to. + validatingProjects: [], // Frame counter for the spinner (incremented by the tick loop). spinFrame: 0, // Most recent error captured by `serve-error`. diff --git a/packages/logger/test/lib/loggers/Serve.js b/packages/logger/test/lib/loggers/Serve.js index 0244f4177e5..70124d3ec20 100644 --- a/packages/logger/test/lib/loggers/Serve.js +++ b/packages/logger/test/lib/loggers/Serve.js @@ -56,6 +56,26 @@ test.serial("stale: Missing parameter", (t) => { }, "Threw with expected error message"); }); +test.serial("validating emits serve-validating", (t) => { + const {serveLogger, statusHandler} = t.context; + serveLogger.validating(["library.a", "library.b"]); + t.is(statusHandler.callCount, 1, "One serve-status event emitted"); + t.deepEqual(statusHandler.getCall(0).args[0], { + level: "info", + status: "serve-validating", + validatingProjects: ["library.a", "library.b"], + }, "Status event has expected payload"); +}); + +test.serial("validating: Missing parameter", (t) => { + const {serveLogger} = t.context; + t.throws(() => { + serveLogger.validating(); + }, { + message: "loggers/Serve#validating: Missing or incorrect validatingProjects parameter", + }, "Threw with expected error message"); +}); + test.serial("building emits serve-building", (t) => { const {serveLogger, statusHandler} = t.context; serveLogger.building(); @@ -129,6 +149,18 @@ test.serial("No event listener: stale", (t) => { t.is(logHandler.callCount, 0, "No log event emitted"); }); +test.serial("No event listener: validating", (t) => { + const {serveLogger, statusHandler, logHandler, logStub} = t.context; + process.off(ServeLogger.SERVE_STATUS_EVENT_NAME, statusHandler); + serveLogger.validating(["library.a", "library.b"]); + t.is(logStub.callCount, 1, "_log got called once"); + t.is(logStub.getCall(0).args[0], "info", "Logged with expected log-level"); + t.is(logStub.getCall(0).args[1], + "Validating caches for: library.a, library.b", + "Logged expected message"); + t.is(logHandler.callCount, 0, "No log event emitted"); +}); + test.serial("No event listener: building", (t) => { const {serveLogger, statusHandler, logHandler, logStub} = t.context; process.off(ServeLogger.SERVE_STATUS_EVENT_NAME, statusHandler); diff --git a/packages/logger/test/lib/writers/InteractiveConsole.js b/packages/logger/test/lib/writers/InteractiveConsole.js index 0cd0b2833b5..c15a8a50751 100644 --- a/packages/logger/test/lib/writers/InteractiveConsole.js +++ b/packages/logger/test/lib/writers/InteractiveConsole.js @@ -535,6 +535,40 @@ test.serial("serve-status: serve-stale without a payload falls back to an empty writer.disable(); }); +test.serial("serve-status: serve-validating records projects and transitions to VALIDATING", (t) => { + const {writer, stderr} = createWriter(); + process.emit("ui5.serve-status", { + status: "serve-validating", + validatingProjects: ["library.a", "library.b"], + }); + const state = writer._getStateForTest().build; + t.is(state.state, STATES.VALIDATING); + t.deepEqual(state.validatingProjects, ["library.a", "library.b"]); + const tail = stripAnsi(stderr.writes.slice(-5).join("")); + t.regex(tail, /validating/); + writer.disable(); +}); + +test.serial("serve-status: serve-validating without a payload falls back to an empty list", (t) => { + const {writer} = createWriter(); + process.emit("ui5.serve-status", {status: "serve-validating"}); + t.deepEqual(writer._getStateForTest().build.validatingProjects, []); + writer.disable(); +}); + +test.serial("serve-status: transitions away from VALIDATING clear validatingProjects", (t) => { + const {writer} = createWriter(); + process.emit("ui5.serve-status", { + status: "serve-validating", + validatingProjects: ["library.a"], + }); + t.deepEqual(writer._getStateForTest().build.validatingProjects, ["library.a"]); + process.emit("ui5.serve-status", {status: "serve-ready"}); + t.deepEqual(writer._getStateForTest().build.validatingProjects, [], + "validatingProjects cleared on transition away from VALIDATING"); + writer.disable(); +}); + test.serial("serve-status: serve-error coerces a non-Error payload to a string", (t) => { const {writer} = createWriter(); process.emit("ui5.serve-status", {status: "serve-error", error: "plain string failure"}); @@ -634,6 +668,25 @@ test.serial("spinner tick advances spinFrame while BUILDING", (t) => { writer.disable(); }); +test.serial("spinner tick advances spinFrame while VALIDATING", (t) => { + const clock = sinon.useFakeTimers(); + t.teardown(() => clock.restore()); + + const {writer, stderr} = createWriter(); + process.emit("ui5.serve-status", { + status: "serve-validating", + validatingProjects: ["library.a"], + }); + const frameBefore = writer._getStateForTest().build.spinFrame; + stderr.writes.length = 0; + clock.tick(150); + const frameAfter = writer._getStateForTest().build.spinFrame; + t.true(frameAfter > frameBefore, "spinFrame advanced on tick"); + t.true(stderr.writes.length > 0, "tick triggered a re-render"); + writer.disable(); +}); + + // ---- process.stdout / process.stderr interception --------------------------- // InteractiveConsole replaces process.stdout.write / process.stderr.write while // active so writes that bypass @ui5/logger (custom tasks, third-party libs) get diff --git a/packages/logger/test/lib/writers/interactiveConsole/render.js b/packages/logger/test/lib/writers/interactiveConsole/render.js index 01bbc9ae54a..64b001d8fae 100644 --- a/packages/logger/test/lib/writers/interactiveConsole/render.js +++ b/packages/logger/test/lib/writers/interactiveConsole/render.js @@ -253,6 +253,30 @@ test("renderBuildRegion: stale state includes 'files changed' hint", (t) => { t.regex(plain, /Status\s+.+?\s+stale\s+·\s+files changed/); }); +test("renderBuildRegion: validating state includes 'checking dependency caches' hint", (t) => { + const state = createBuildState(); + transitionTo(state, STATES.VALIDATING); + const plain = renderBuildRegion(state).map(stripAnsi).join("\n"); + t.regex(plain, /Status\s+.+?\s+validating cache\s+·\s+checking dependency caches/); + t.notRegex(plain, /project/, "no counter fragment when validatingProjects is empty"); +}); + +test("renderBuildRegion: validating state appends singular project count", (t) => { + const state = createBuildState(); + state.validatingProjects = ["library.a"]; + transitionTo(state, STATES.VALIDATING); + const plain = renderBuildRegion(state).map(stripAnsi).join("\n"); + t.regex(plain, /·\s+1 project(?!s)/); +}); + +test("renderBuildRegion: validating state appends plural project count", (t) => { + const state = createBuildState(); + state.validatingProjects = ["library.a", "library.b"]; + transitionTo(state, STATES.VALIDATING); + const plain = renderBuildRegion(state).map(stripAnsi).join("\n"); + t.regex(plain, /·\s+2 projects/); +}); + test("renderBuildRegion: building state renders project counter + project + task", (t) => { const state = createBuildState(); beginBuild(state, ["proj-a", "my.app", "proj-c"]); @@ -375,6 +399,7 @@ const COLOR_BY_STATE = [ {state: STATES.READY, wrap: (x) => chalk.green(x), word: "ready", name: "green"}, {state: STATES.STALE, wrap: (x) => chalk.yellow(x), word: "stale", name: "yellow"}, {state: STATES.BUILDING, wrap: (x) => chalk.yellow(x), word: "building", name: "yellow"}, + {state: STATES.VALIDATING, wrap: (x) => chalk.cyan(x), word: "validating cache", name: "cyan"}, {state: STATES.ERROR, wrap: (x) => chalk.red(x), word: "error", name: "red"}, ]; for (const {state: s, wrap, word, name} of COLOR_BY_STATE) { diff --git a/packages/logger/test/lib/writers/interactiveConsole/state/build.js b/packages/logger/test/lib/writers/interactiveConsole/state/build.js index f7977093d2a..62598ccc30b 100644 --- a/packages/logger/test/lib/writers/interactiveConsole/state/build.js +++ b/packages/logger/test/lib/writers/interactiveConsole/state/build.js @@ -18,6 +18,7 @@ test("createBuildState: starts in INITIAL with empty counters", (t) => { t.is(state.totalProjects, 0); t.deepEqual(state.projectOrder, []); t.deepEqual(state.changedProjects, []); + t.deepEqual(state.validatingProjects, []); t.is(state.spinFrame, 0); t.is(state.errorMessage, ""); t.is(state.lastBuildHrtime, null); diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 5eae69b93cb..78dc1eebe95 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -2,7 +2,7 @@ import EventEmitter from "node:events"; import {createReaderCollectionPrioritized} from "@ui5/fs/resourceFactory"; import BuildReader from "./BuildReader.js"; import WatchHandler from "./helpers/WatchHandler.js"; -import {SourceChangedDuringBuildError} from "./cache/ProjectBuildCache.js"; +import {isAbortError} from "./helpers/abort.js"; import {getLogger} from "@ui5/logger"; import ServeLogger from "@ui5/logger/internal/loggers/Serve"; const log = getLogger("build:BuildServer"); @@ -13,9 +13,10 @@ const SOURCES_CHANGED_DEBOUNCE_MS = 100; // The server's lifecycle state. Mutated exclusively through #setState. const SERVER_STATES = Object.freeze({ - IDLE: "idle", // No pending requests, no recent changes. + IDLE: "idle", // No pending requests, no recent changes, no unvalidated caches. STALE: "stale", // Pending changes / pending requests, queue not yet flushed. BUILDING: "building", // A build is in flight. + VALIDATING: "validating", // A background cache-validation pass is in flight. ERROR: "error", // Last build cycle failed. }); @@ -66,6 +67,11 @@ class BuildServer extends EventEmitter { // Server lifecycle state. Starts as `null` so the first #setState call // always emits the initial state. #serverState = null; + // Background cache validation state. `#activeValidation` is the promise of the + // currently running validation pass (or null when idle); `#validationAbort` + // is its controller, used to preempt validation when a real build is requested. + #activeValidation = null; + #validationAbort = null; /** * Creates a new BuildServer instance @@ -180,6 +186,8 @@ class BuildServer extends EventEmitter { clearTimeout(this.#sourcesChangedTimeout); await this.#watchHandler.destroy(); try { + // Cancel any running background validation pass and wait for it to settle. + await this.#stopActiveValidation("Server destroyed"); if (this.#activeBuild) { // Await active build to finish await this.#activeBuild; @@ -256,6 +264,20 @@ class BuildServer extends EventEmitter { const {promise, resolve, reject} = Promise.withResolvers(); projectBuildStatus.addReaderRequest({resolve, reject}); + // Skip the build-request enqueue while a background validation pass owns the + // project. If validation finds the cache fresh, it resolves the queued reader + // via setReader; if it finds the cache stale, #runBackgroundValidation enqueues + // a build for it (since pending reader requests can't wait for the next ad-hoc + // reader request). Enqueuing here unconditionally would fire #triggerRequestQueue's + // 10 ms timer, abort the validation pass mid-flight, and cause a needless + // VALIDATING → BUILDING → VALIDATING flicker even when validation would have + // served the request itself. + if (projectBuildStatus.isValidating()) { + log.verbose(`Reader for project '${projectName}' is validating. ` + + `Waiting on validation pass.`); + return promise; + } + log.verbose(`Reader for project '${projectName}' is not fresh. Enqueuing build request.`); this.#enqueueBuild(projectName); return promise; @@ -332,7 +354,12 @@ class BuildServer extends EventEmitter { } // If the server is currently idle, surface the new STALE state right away. - if (this.#serverState === SERVER_STATES.IDLE) { + // During VALIDATING, the same shortcut applies: the change is already registered + // here, so reporting VALIDATING any longer would mislead consumers. The validation + // pass's finally clause guards on `#serverState === VALIDATING` and bails when the + // state has moved on, so there's no double-transition risk. + if (this.#serverState === SERVER_STATES.IDLE || + this.#serverState === SERVER_STATES.VALIDATING) { this.#setState(SERVER_STATES.STALE); } @@ -389,17 +416,39 @@ class BuildServer extends EventEmitter { if (this.#processBuildRequestsTimeout) { clearTimeout(this.#processBuildRequestsTimeout); } - this.#processBuildRequestsTimeout = setTimeout(() => { + this.#processBuildRequestsTimeout = setTimeout(async () => { + // Abort any in-flight background validation pass so the build can claim + // the builder's "buildIsRunning" lock. Validation will be re-scheduled + // after the build cycle drains. + await this.#stopActiveValidation("Build request received"); + if (this.#destroyed) { + return; + } const cycleStart = process.hrtime(); this.#setState(SERVER_STATES.BUILDING); this.#processBuildRequests().then(() => { const hrtime = process.hrtime(cycleStart); + // #processBuildRequests captures individual build failures on the + // server state and continues the queue, so it resolves normally + // even when the build errored. Leave the state as-is (ERROR) and + // skip the post-build validation pass — the pass would surface + // yet more Force-mode / cache errors on top of the build error + // that already fired. + if (this.#serverState === SERVER_STATES.ERROR) { + return; + } const staleProjects = this.#getStaleProjectNames(); log.verbose(`Build cycle done. Stale projects: `+ `${staleProjects.length} (${staleProjects.join(", ")})`); if (staleProjects.length === 0) { this.#setState(SERVER_STATES.IDLE, {hrtime}); - } else { + return; + } + // Some projects are still non-FRESH. Try to validate any that are merely + // INITIAL (cache validity unknown but possibly fresh) in the background. + // If a pass actually starts, the transition to VALIDATING happens inside + // #scheduleBackgroundValidation; otherwise we fall back to STALE. + if (!this.#scheduleBackgroundValidation({hrtime})) { this.#setState(SERVER_STATES.STALE, {hrtime, staleProjects}); } }).catch((err) => { @@ -456,7 +505,7 @@ class BuildServer extends EventEmitter { const projectBuildStatus = this.#projectBuildStatus.get(projectName); projectBuildStatus.setReader(project.getReader({style: "runtime"})); }).catch((err) => { - if (err instanceof AbortBuildError || err instanceof SourceChangedDuringBuildError) { + if (isAbortError(err)) { log.info("Build aborted"); log.verbose(`Projects affected by abort: ${projectsToBuild.join(", ")}`); // Build was aborted - do not log as error @@ -518,6 +567,160 @@ class BuildServer extends EventEmitter { return stale; } + /** + * Aborts the in-flight background validation pass (if any) and awaits its settlement. + * Swallows the resulting abort rejection — callers use this to make room for a build + * or to drain on destroy, neither of which surfaces validation errors. + * + * @param {string} reason Reason forwarded to AbortBuildError for verbose logging. + */ + async #stopActiveValidation(reason) { + if (!this.#activeValidation) { + return; + } + this.#validationAbort?.abort(new AbortBuildError(reason)); + try { + await this.#activeValidation; + } catch (err) { + // Expected — validation rejects with an AbortBuildError on cancellation. + log.verbose(`Background validation settled (${reason}): ${err?.message ?? err}`); + } + } + + /** + * Kick off a background cache-validation pass for projects that have never been built or + * validated yet. Runs as a fire-and-forget promise; the result is tracked on + * #activeValidation so {@link #triggerRequestQueue} and {@link #destroy} can + * abort it. + * + * Drives the server lifecycle through VALIDATING → IDLE/STALE on completion. Caller + * supplies the post-build hrtime so the BUILDING → VALIDATING transition can emit a + * buildDone event with the correct duration. + * + * Idempotent while a validation pass is already in flight. Skipped while a build is + * active — the post-build cycle-end hook will schedule the next pass. + * + * @param {object} [opts] + * @param {Array} [opts.hrtime] Build hrtime to forward to the VALIDATING state + * transition. Only relevant when called from a post-build cycle-end hook. + * @returns {boolean} True if a validation pass was actually scheduled, false otherwise. + * When false, callers may want to transition the server to STALE explicitly. + */ + #scheduleBackgroundValidation({hrtime} = {}) { + if (this.#destroyed || this.#activeValidation || this.#activeBuild) { + return false; + } + const projectsToValidate = []; + const projectAbortSignals = []; + for (const [name, status] of this.#projectBuildStatus) { + if (status.isInitial()) { + projectsToValidate.push(name); + // Capture the per-project abort signal now, before any invalidate() rotates it. + // A source change for any of these projects fires invalidate(), which aborts the + // signal and cancels the in-flight validation pass via the composite signal below. + projectAbortSignals.push(status.getAbortSignal()); + } + } + if (projectsToValidate.length === 0) { + return false; + } + log.verbose(`Scheduling background cache validation for projects: ${projectsToValidate.join(", ")}`); + this.#setState(SERVER_STATES.VALIDATING, {hrtime, validatingProjects: projectsToValidate}); + this.#validationAbort = new AbortController(); + const signal = AbortSignal.any([this.#validationAbort.signal, ...projectAbortSignals]); + this.#activeValidation = this.#runBackgroundValidation(projectsToValidate, signal) + .catch((err) => { + if (isAbortError(err)) { + log.verbose(`Background cache validation aborted: ${err?.message ?? err}`); + return; + } + // Non-abort failure: mirror the build error path so consumers (the banner, + // integration tests, the yargs fail-handler) can react. The ERROR transition + // here doubles as the signal to the finally clause to skip its post-pass + // IDLE/STALE transition. + this.#setState(SERVER_STATES.ERROR, {error: err}); + this.emit("error", err); + }) + .finally(() => { + this.#activeValidation = null; + this.#validationAbort = null; + // If a build is queued or running, leave state to #triggerRequestQueue's + // BUILDING transition. Otherwise (and unless validation failed — in which + // case the catch above has already settled state on ERROR) reflect the + // outcome of the validation pass: fully fresh → IDLE, anything else → STALE. + if (this.#destroyed || this.#activeBuild || + this.#pendingBuildRequest.size > 0 || + this.#serverState === SERVER_STATES.BUILDING || + this.#serverState === SERVER_STATES.ERROR) { + return; + } + const staleProjects = this.#getStaleProjectNames(); + if (staleProjects.length === 0) { + this.#setState(SERVER_STATES.IDLE); + } else { + this.#setState(SERVER_STATES.STALE, {staleProjects}); + } + }); + return true; + } + + async #runBackgroundValidation(projectNames, signal) { + try { + await this.#projectBuilder.validateCaches({ + projects: projectNames, + signal, + willValidate: (projectName) => { + // Claim the project for validation. If the project is no longer INITIAL + // (invalidated mid-pass, or a build started concurrently), the transition is + // a no-op and validateCache below will run without claiming the project; + // the subsequent setReader call sees a non-VALIDATING state and drops. + const projectBuildStatus = this.#projectBuildStatus.get(projectName); + projectBuildStatus?.markValidating(); + }, + }, (projectName, project, projectBuildContext, usesCache) => { + if (signal.aborted) { + return; + } + const projectBuildStatus = this.#projectBuildStatus.get(projectName); + if (!projectBuildStatus) { + return; + } + if (!usesCache) { + // Cache is stale; leave the project in INITIAL so a future reader request + // triggers a real build. We don't pre-emptively rebuild dependencies. + projectBuildStatus.releaseValidating(); + return; + } + log.verbose(`Background validation: marking project '${projectName}' as fresh`); + // setReader is a no-op if state isn't VALIDATING (e.g. invalidated mid-validation + // or claimed by a build) — the cycle-end logic will re-schedule or rebuild. + projectBuildStatus.setReader(project.getReader({style: "runtime"})); + }); + } finally { + // Whether the pass completed normally, was aborted, or threw, ensure no project + // is left stuck in VALIDATING — otherwise the next scheduleBackgroundValidation + // pass (which picks up only INITIAL projects) would skip them and a reader request + // would still incur the lazy validation cost. + // + // Reader requests issued during VALIDATING skip #enqueueBuild (see + // #getReaderForProject) so the validation pass can run to completion without being + // aborted by its own queue tick. Pick up the slack here: any project the pass + // didn't promote to FRESH but which has callers waiting on a reader must now be + // built. #enqueueBuild is idempotent, so re-enqueueing one already scheduled by + // the watcher or a concurrent caller is harmless. + for (const projectName of projectNames) { + const status = this.#projectBuildStatus.get(projectName); + if (!status) { + continue; + } + status.releaseValidating(); + if (!status.isFresh() && status.hasPendingReaderRequests()) { + this.#enqueueBuild(projectName); + } + } + } + } + /** * Single source of truth for the server lifecycle state. Mutates * #serverState and emits the matching ServeLogger event for the @@ -528,21 +731,25 @@ class BuildServer extends EventEmitter { * * * @param {string} next One of the SERVER_STATES values. * @param {object} [opts] * @param {Array} [opts.hrtime] Build duration as a [seconds, nanoseconds] * tuple produced by process.hrtime(start); required when leaving - * BUILDING for IDLE/STALE. + * BUILDING for IDLE/STALE/VALIDATING. * @param {string[]} [opts.staleProjects] Stale project names; required when transitioning to STALE. + * @param {string[]} [opts.validatingProjects] Names of projects undergoing background cache + * validation; required when transitioning to VALIDATING. * @param {Error} [opts.error] Error instance; required when transitioning to ERROR. */ - #setState(next, {hrtime, staleProjects, error} = {}) { + #setState(next, {hrtime, staleProjects, validatingProjects, error} = {}) { if (this.#serverState === next) { return; } @@ -563,6 +770,9 @@ class BuildServer extends EventEmitter { case SERVER_STATES.BUILDING: this.#serveLogger.building(); break; + case SERVER_STATES.VALIDATING: + this.#serveLogger.validating(validatingProjects ?? []); + break; case SERVER_STATES.ERROR: this.#serveLogger.serveError(error); break; @@ -573,6 +783,7 @@ class BuildServer extends EventEmitter { const PROJECT_STATES = Object.freeze({ INITIAL: "initial", INVALIDATED: "invalidated", + VALIDATING: "validating", BUILDING: "building", FRESH: "fresh", }); @@ -617,11 +828,53 @@ class ProjectBuildStatus { * project mid-build, the state flips back to INVALIDATED via * invalidate(), which causes setReader() to drop * the late-arriving result. + * + * Unlike markValidating, this transition is unconditional — + * #processBuildRequests always intends to claim the project + * regardless of its prior state. The asymmetry is deliberate; the call sites + * pull from #pendingBuildRequest, which a FRESH project never + * enters (see #getReaderForProject). */ markBuilding() { this.#state = PROJECT_STATES.BUILDING; } + /** + * Marks the project as being validated by a background cache-validation pass. + * Only takes effect for projects in INITIAL state — projects that have been + * invalidated, are already being built, or are FRESH must not be claimed by + * a validation pass. + * + * Used in place of {@link #markBuilding} for the validation flow so the state + * accurately reflects that no build work is taking place. setReader() + * accepts both VALIDATING and BUILDING as legitimate prior states, so the + * validation callback can promote a project to FRESH the same way a real build + * does. If a source change invalidates the project mid-validation, + * invalidate() flips VALIDATING → INVALIDATED and aborts the + * per-project signal so the validation pass cancels. + * + * @returns {boolean} True if the transition happened, false otherwise. + */ + markValidating() { + if (this.#state !== PROJECT_STATES.INITIAL) { + return false; + } + this.#state = PROJECT_STATES.VALIDATING; + return true; + } + + /** + * Reverts a VALIDATING project back to INITIAL — used when validation found the + * cache to be stale and the project should remain lazy. No-op for any other state + * (e.g. when invalidate() already moved the project to INVALIDATED mid-validation, + * or a build claimed it in the meantime). + */ + releaseValidating() { + if (this.#state === PROJECT_STATES.VALIDATING) { + this.#state = PROJECT_STATES.INITIAL; + } + } + abortBuild(reason) { this.#abortController.abort(reason); } @@ -634,14 +887,22 @@ class ProjectBuildStatus { return this.#state === PROJECT_STATES.FRESH; } + isInitial() { + return this.#state === PROJECT_STATES.INITIAL; + } + + isValidating() { + return this.#state === PROJECT_STATES.VALIDATING; + } + getReader() { return this.#reader; } setReader(reader) { - if (this.#state !== PROJECT_STATES.BUILDING) { - // Project was re-invalidated mid-build; drop the stale reader and let - // the cycle-end logic re-queue a fresh build. + if (this.#state !== PROJECT_STATES.BUILDING && this.#state !== PROJECT_STATES.VALIDATING) { + // Project was re-invalidated mid-build (or mid-validation); drop the stale reader + // and let the cycle-end logic re-queue a fresh build. return; } this.#reader = reader; @@ -657,6 +918,10 @@ class ProjectBuildStatus { this.#readerQueue.push(promiseResolvers); } + hasPendingReaderRequests() { + return this.#readerQueue.length > 0; + } + rejectReaderRequests(error) { this.#state = PROJECT_STATES.INVALIDATED; for (const {reject} of this.#readerQueue) { diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index bbc8380755f..9fa62fd95b3 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -5,6 +5,7 @@ import composeProjectList from "./helpers/composeProjectList.js"; import BuildContext from "./helpers/BuildContext.js"; import prettyHrtime from "pretty-hrtime"; import OutputStyleEnum from "./helpers/ProjectBuilderOutputStyle.js"; +import {isAbortError} from "./helpers/abort.js"; /** * @public @@ -189,6 +190,42 @@ class ProjectBuilder { return await this.#build(requestedProjects, projectBuiltCallback, signal); } + /** + * Validate the build cache for a set of projects without actually building any of them. + * + * Intended to be used by long-running consumers (such as the + * [BuildServer]{@link @ui5/project/build/BuildServer}) to proactively determine whether + * a cached build result can be reused for a project. Walks the dependency graph in the + * same order as {@link #build}, so dependencies are validated before their dependents + * and resource changes propagate correctly. + * + * For each requested project, the given callback is invoked with the validation result + * once that project has been validated. The result indicates whether a cached build + * result is available and can be used (true) or whether a build would be + * required (false). + * + * Like {@link #build}, this method is mutually exclusive with itself and with other + * build operations on the same builder instance. Source change propagation via + * {@link #resourcesChanged} is blocked while validation is running. + * + * @public + * @param {object} parameters Parameters + * @param {string[]} parameters.projects Names of projects to validate + * @param {AbortSignal} [parameters.signal] Signal to abort the validation + * @param {Function} [parameters.willValidate] + * Hook invoked synchronously just before each project's validateCache + * call. Receives (projectName). Return value is ignored. Use this to + * claim the project state in the caller (e.g. transition to a "validating" + * lifecycle state). + * @param {Function} [projectValidatedCallback] + * Callback invoked after each requested project has been validated. + * Receives (projectName, project, projectBuildContext, usesCache). + * @returns {Promise} Promise resolving with the names of all processed projects + */ + async validateCaches({projects, signal, willValidate}, projectValidatedCallback) { + return await this.#validate(projects, willValidate, projectValidatedCallback, signal); + } + /** * Executes a project build, including all necessary or requested dependencies, and writes * the result to the given target directory. @@ -403,11 +440,7 @@ class ProjectBuilder { // is not a build failure. The BuildServer turns it into a re-queue. // Log at verbose so we don't shout an error at the user for what // is a normal mid-build source change in `ui5 serve`. - const aborted = signal?.aborted === true || - err?.name === "AbortBuildError" || - err?.name === "SourceChangedDuringBuildError" || - err?.name === "AbortError"; - if (aborted) { + if (isAbortError(err, signal)) { this.#log.verbose(`Build aborted: ${err?.message ?? err}`); } else { this.#log.error(`Build failed`); @@ -425,6 +458,112 @@ class ProjectBuilder { } } + /** + * Internal validation implementation that mirrors {@link #build} but only validates caches. + * + * Loads the project build contexts, walks them in dependency-first order, and asks each + * context to validate its build cache. The given callback is invoked after each requested + * project has been validated with a flag indicating whether the cached build result is + * usable. + * + * @param {string[]} requestedProjects Array of project names to validate + * @param {Function} [willValidate] Hook invoked just before each project's validateCache call + * @param {Function} [projectValidatedCallback] + * Callback invoked after each requested project has been validated + * @param {AbortSignal} [signal] Signal to abort the validation + * @returns {Promise} Promise resolving with array of processed project names + * @throws {Error} If a build is already running + */ + async #validate(requestedProjects, willValidate, projectValidatedCallback, signal) { + if (this.#buildIsRunning) { + throw new Error("A build is already running"); + } + this.#buildIsRunning = true; + try { + // Initialize (or reuse) the build contexts for the requested projects and their + // transitive build-time dependencies, mirroring what #build does. Validation is + // commonly invoked before any project has been built in this process — e.g. when + // `ui5 serve` runs a post-(initial-)build cache-validation pass over dependencies + // the initial build skipped — so it must be able to initialize cold contexts and + // source indices itself rather than only validating ones a prior build warmed up. + // + // This is race-free with `#revalidateSourceIndex`: the BuildServer's file watcher + // is up before any builds run, so file changes between this FS scan and a later + // build flow through `projectSourcesChanged` → `#flushPendingChanges` → + // `#updateSourceIndex` at the next build's start, leaving the source index + // consistent by the time `#revalidateSourceIndex` runs at build end. + const projectBuildContexts = + await this._buildContext.getRequiredProjectContexts(requestedProjects); + if (projectBuildContexts.size === 0) { + this.#log.verbose(`No projects to validate`); + return []; + } + this.#log.verbose(`Validating caches for projects: ${Array.from(projectBuildContexts.keys()).join(", ")}`); + + // requestedProjects gates the willValidate + projectValidatedCallback hooks. + // The queue walks the full transitive closure (cold contexts included), so + // checking membership per project is O(N) on an array — hoist to a Set. + const requestedSet = new Set(requestedProjects); + + // Build queue based on graph depth-first search to ensure that dependencies + // are validated before their dependents, so propagated resource changes reach + // dependents before those dependents validate themselves. + const queue = []; + const processedProjectNames = []; + for (const {project} of this._graph.traverseDependenciesDepthFirst(true)) { + const projectName = project.getName(); + const projectBuildContext = projectBuildContexts.get(projectName); + if (projectBuildContext) { + queue.push(projectBuildContext); + processedProjectNames.push(projectName); + } + } + + try { + while (queue.length) { + signal?.throwIfAborted(); + const projectBuildContext = queue.shift(); + const project = projectBuildContext.getProject(); + const projectName = project.getName(); + const isRequested = requestedSet.has(projectName); + + if (willValidate && isRequested) { + willValidate(projectName); + } + + let usesCache; + if (!projectBuildContext.possiblyRequiresBuild()) { + // Build manifest present (or cache already known fresh) -> no validation needed + usesCache = true; + } else { + const valStart = performance.now(); + usesCache = await projectBuildContext.validateCache(); + if (this.#log.isLevelEnabled("perf")) { + this.#log.perf( + `validateCache for ${projectName} ` + + `completed in ${(performance.now() - valStart).toFixed(2)} ms ` + + `(usesCache=${usesCache})`); + } + } + + if (projectValidatedCallback && isRequested) { + await projectValidatedCallback(projectName, project, projectBuildContext, usesCache); + } + } + } catch (err) { + if (isAbortError(err, signal)) { + this.#log.verbose(`Cache validation aborted: ${err?.message ?? err}`); + } else { + this.#log.error(`Cache validation failed: ${err?.message ?? err}`); + } + throw err; + } + return processedProjectNames; + } finally { + this.#buildIsRunning = false; + } + } + /** * Build a single project * diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index 58702ed4bcf..b3865c9b46a 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -171,6 +171,24 @@ export default class ProjectBuildCache { this.#currentDependencyReader = dependencyReader; + return this.validateCache(dependencyReader); + } + + /** + * Validates the current build cache state without performing build-prep side effects. + * + * Flushes any pending source/dependency changes, refreshes dependency indices on first use, + * and attempts to locate a cached result stage. Safe to call independently of a build attempt + * (e.g. to check whether the cache is stale). + * + * @public + * @param {@ui5/fs/AbstractReader} dependencyReader Reader for dependency resources, used to + * refresh dependency indices when required + * @returns {Promise} + * Array of changed resource paths since last build, true if cache is fresh, false + * if cache is empty + */ + async validateCache(dependencyReader) { // When cache=Off, don't validate or use result cache if (this.#cacheMode === Cache.Off) { log.verbose(`Cache is in "Off" mode for project ${this.#project.getName()}. ` + @@ -210,7 +228,7 @@ export default class ProjectBuildCache { if (this.#combinedIndexState === INDEX_STATES.REQUIRES_UPDATE) { const flushStart = performance.now(); - const changesDetected = await this.#flushPendingChanges(); + const changesDetected = await this.#flushPendingChanges(dependencyReader); if (changesDetected) { this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION; // Force mode: Fail immediately if changes were detected @@ -252,9 +270,10 @@ export default class ProjectBuildCache { /** * Processes changed resources since last build, updating indices and invalidating tasks as needed * + * @param {@ui5/fs/AbstractReader} dependencyReader Reader for dependency resources * @returns {Promise} */ - async #flushPendingChanges() { + async #flushPendingChanges(dependencyReader) { if (this.#changedProjectSourcePaths.length === 0 && this.#changedDependencyResourcePaths.length === 0) { return; @@ -279,7 +298,7 @@ export default class ProjectBuildCache { .filter((taskCache) => taskCache.hasDependencyRequests()); await Promise.all(tasksWithDepRequests.map(async (taskCache) => { const changed = await taskCache - .updateDependencyIndices(this.#currentDependencyReader, this.#changedDependencyResourcePaths); + .updateDependencyIndices(dependencyReader, this.#changedDependencyResourcePaths); if (changed) { depIndicesChanged = true; } diff --git a/packages/project/lib/build/helpers/ProjectBuildContext.js b/packages/project/lib/build/helpers/ProjectBuildContext.js index 332b2c43065..f73388525b8 100644 --- a/packages/project/lib/build/helpers/ProjectBuildContext.js +++ b/packages/project/lib/build/helpers/ProjectBuildContext.js @@ -269,20 +269,52 @@ class ProjectBuildContext { * True if a valid cache was found and is being used. False otherwise (indicating a build is required). */ async prepareProjectBuildAndValidateCache() { - const readerStart = performance.now(); + return this.#runCacheValidation( + "prepareProjectBuildAndValidateCache", + (cache, depReader) => cache.prepareProjectBuildAndValidateCache(depReader)); + } + + /** + * Validates the current build cache without performing build-prep side effects. + * + * Creates a dependency reader and asks the build cache whether it is stale, + * propagating any detected resource changes to dependents. + * + * @returns {Promise} + * True if a valid cache was found and is being used. False otherwise. + */ + async validateCache() { + return this.#runCacheValidation( + "validateCache", + (cache, depReader) => cache.validateCache(depReader)); + } + + /** + * Shared implementation for the two cache-validation entry points: fetches a fresh + * dependency reader, invokes the supplied cache call, and propagates any changed + * resource paths to dependents. + * + * @param {string} label Method name used for perf logging + * @param {Function} call Callback receiving (cache, depReader); returns boolean or string[] + * @returns {Promise} + * True if a valid cache was found and is being used. False otherwise. + */ + async #runCacheValidation(label, call) { + const perfEnabled = this._log.isLevelEnabled("perf"); + const readerStart = perfEnabled ? performance.now() : 0; const depReader = await this.getTaskRunner().getDependenciesReader( await this.getTaskRunner().getRequiredDependencies(), true, // Force creation of new reader since project readers might have changed during their (re-)build ); - if (this._log.isLevelEnabled("perf")) { + if (perfEnabled) { this._log.perf( `getDependenciesReader completed in ${(performance.now() - readerStart).toFixed(2)} ms`); } - const cacheStart = performance.now(); - const boolOrChangedPaths = await this.getBuildCache().prepareProjectBuildAndValidateCache(depReader); - if (this._log.isLevelEnabled("perf")) { + const cacheStart = perfEnabled ? performance.now() : 0; + const boolOrChangedPaths = await call(this.getBuildCache(), depReader); + if (perfEnabled) { this._log.perf( - `ProjectBuildCache.prepareProjectBuildAndValidateCache completed in ` + + `ProjectBuildCache.${label} completed in ` + `${(performance.now() - cacheStart).toFixed(2)} ms`); } if (Array.isArray(boolOrChangedPaths)) { diff --git a/packages/project/lib/build/helpers/abort.js b/packages/project/lib/build/helpers/abort.js new file mode 100644 index 00000000000..1da47dd4296 --- /dev/null +++ b/packages/project/lib/build/helpers/abort.js @@ -0,0 +1,18 @@ +/** + * Recognized error names that signal a cooperative abort — a triggered + * AbortSignal or a mid-build source change — rather than a genuine build / + * validation failure. Callers pass the signal so a raced abort (signal + * aborted, error already thrown from something else) still classifies as + * an abort. + * + * @param {Error} err + * @param {AbortSignal} [signal] Optional signal; treated as aborted when its + * `aborted` flag is set even if `err` doesn't carry a recognized name. + * @returns {boolean} + */ +export function isAbortError(err, signal) { + return signal?.aborted === true || + err?.name === "AbortBuildError" || + err?.name === "SourceChangedDuringBuildError" || + err?.name === "AbortError"; +} diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 8084ca7f65d..576c7b26923 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -5,6 +5,20 @@ import esmock from "esmock"; // Note: These tests are focused on the debounce behavior of the `sourcesChanged` event. // The general BuildServer functionality is tested by the integration test at ./BuildServer.integration.js +// Drain microtasks (zero-ticks) until `predicate` is true for some entry in +// `statusEvents`. Use in place of an arbitrary number of `await clock.tickAsync(0)` +// calls when waiting for an async chain to settle — the number of microtask hops +// is an implementation detail that shifts every time a new `await` is added to +// `#runBackgroundValidation` or its finally clause. +async function drainUntil(clock, statusEvents, predicate, {maxTicks = 100} = {}) { + for (let i = 0; i < maxTicks; i++) { + if (statusEvents.some(predicate)) return; + await clock.tickAsync(0); + } + throw new Error(`drainUntil: predicate not satisfied after ${maxTicks} ticks; ` + + `events: ${statusEvents.map((e) => e.status).join(", ")}`); +} + test.beforeEach(async (t) => { const sinon = t.context.sinon = sinonGlobal.createSandbox(); t.context.clock = sinon.useFakeTimers(); @@ -28,6 +42,7 @@ test.beforeEach(async (t) => { t.context.projectBuilder = { closeCacheManager: sinon.stub(), resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().resolves([]), }; // on()/watch() are stubbed to swallow BuildServer#initWatcher's wiring calls; the tests @@ -294,3 +309,462 @@ test.serial("serve-status: build failure emits serveError and no orphan building const errorIdx = seq.indexOf("serve-error"); t.true(errorIdx > lastBuildingIdx, "serve-error follows serve-building"); }); + +// When a build cycle drains with INITIAL-state dependencies left over, the server +// must transition BUILDING → VALIDATING (not STALE), then VALIDATING → IDLE +// once the background validation pass confirms every cache is fresh. +test.serial( + "serve-status: BUILDING → VALIDATING → IDLE when dependencies validate clean", async (t) => { + const {BuildServer, sinon, clock} = t.context; + + // Augment the graph with a single library dependency so the build cycle leaves + // one INITIAL project behind, which is exactly the trigger for VALIDATING. + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* (_projectName) { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + + // validateCaches: report library.x's cache as fresh — usesCache=true triggers the + // promote-to-FRESH path that closes out the validation pass with no stale projects. + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().callsFake(async ({willValidate}, callback) => { + willValidate?.("library.x"); + await callback("library.x", { + getReader: () => ({fakeReader: true}), + }, {}, true); + }), + build: sinon.stub().callsFake((_opts, perProjectCb) => { + perProjectCb("root.project", {getReader: () => ({fakeReader: true})}); + return Promise.resolve(["root.project"]); + }), + }; + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + + // Drain the 10ms request-queue tick → build → validation pass. + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-ready"); + + const seq = statusEvents.map((e) => e.status); + const idxBuilding = seq.indexOf("serve-building"); + const idxBuildDone = seq.indexOf("serve-build-done"); + const idxValidating = seq.indexOf("serve-validating"); + const idxReady = seq.indexOf("serve-ready"); + + t.true(idxBuilding >= 0 && idxBuildDone > idxBuilding && + idxValidating > idxBuildDone && idxReady > idxValidating, + `Expected building → buildDone → validating → ready, got: ${seq.join(", ")}`); + + const validatingEvt = statusEvents[idxValidating]; + t.deepEqual(validatingEvt.validatingProjects, ["library.x"], + "validating event carries the project being validated"); + + // No STALE in between — VALIDATING is the post-build state when caches can still be fresh. + t.false(seq.slice(idxBuildDone, idxReady).includes("serve-stale"), + `No stale emission between buildDone and ready; got: ${seq.join(", ")}`); + }); + +// When background validation finds a stale cache (usesCache=false), the project stays +// INITIAL and the validation pass must end on STALE, not IDLE. +test.serial( + "serve-status: VALIDATING → STALE when validation finds a stale cache", async (t) => { + const {BuildServer, sinon, clock} = t.context; + + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* () { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + + // usesCache=false → project must stay INITIAL → final state is STALE. + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().callsFake(async ({willValidate}, callback) => { + willValidate?.("library.x"); + await callback("library.x", { + getReader: () => ({fakeReader: true}), + }, {}, false); + }), + build: sinon.stub().callsFake((_opts, perProjectCb) => { + perProjectCb("root.project", {getReader: () => ({fakeReader: true})}); + return Promise.resolve(["root.project"]); + }), + }; + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-stale"); + + const seq = statusEvents.map((e) => e.status); + t.true(seq.includes("serve-validating"), `validating emitted; got: ${seq.join(", ")}`); + t.true(seq.indexOf("serve-stale") > seq.indexOf("serve-validating"), + `stale follows validating; got: ${seq.join(", ")}`); + t.false(seq.includes("serve-ready"), + `No ready when validation found stale cache; got: ${seq.join(", ")}`); + }); + +// When validateCaches itself rejects with a non-abort error, the failure must be +// observable: BuildServer emits "error", transitions to SERVER_STATES.ERROR (→ +// serve-error on the status feed), and skips the post-validation IDLE/STALE +// transition so the server doesn't silently look "ready" after a failed pass. +test.serial( + "serve-status: validation failure emits serve-error and skips IDLE/STALE", async (t) => { + const {BuildServer, sinon, clock} = t.context; + + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* () { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + + const validationError = new Error("validateCaches blew up"); + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().rejects(validationError), + build: sinon.stub().callsFake((_opts, perProjectCb) => { + perProjectCb("root.project", {getReader: () => ({fakeReader: true})}); + return Promise.resolve(["root.project"]); + }), + }; + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + + const errorEvents = []; + buildServer.on("error", (err) => errorEvents.push(err)); + + await clock.tickAsync(10); + // Drain microtasks until the validation promise chain + // (catch → setState → finally → terminal catch) has settled. + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + + t.is(errorEvents.length, 1, "BuildServer emitted exactly one error event"); + t.is(errorEvents[0], validationError, "Emitted error is the original rejection"); + + const seq = statusEvents.map((e) => e.status); + t.true(seq.includes("serve-validating"), + `validating emitted before the failure; got: ${seq.join(", ")}`); + t.true(seq.indexOf("serve-error") > seq.indexOf("serve-validating"), + `serve-error follows serve-validating; got: ${seq.join(", ")}`); + // Crucial: a failed validation must NOT settle on ready or stale. + const lastError = seq.lastIndexOf("serve-error"); + t.is(seq.indexOf("serve-ready", lastError + 1), -1, + `No ready after a failed validation; got: ${seq.join(", ")}`); + t.is(seq.indexOf("serve-stale", lastError + 1), -1, + `No stale after a failed validation; got: ${seq.join(", ")}`); + }); + +// A reader request issued while a project is in VALIDATING must NOT enqueue a build +// of its own — that would fire #triggerRequestQueue's 10 ms timer, abort the running +// validation pass, and flicker VALIDATING → BUILDING → VALIDATING for no reason. +// Instead, the request should wait on the validation pass, which resolves it via +// setReader when the cache turns out fresh. +test.serial( + "serve-status: reader request during VALIDATING does not enqueue a redundant build", async (t) => { + const {sinon, clock} = t.context; + + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* () { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + + // Hold the validation pass open so the reader request can land mid-VALIDATING. + let releaseValidation; + const validationGate = new Promise((resolve) => { + releaseValidation = resolve; + }); + + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().callsFake(async ({willValidate}, callback) => { + await willValidate?.("library.x"); + await validationGate; + await callback("library.x", { + getReader: () => ({fakeReader: true}), + }, {}, true); + }), + build: sinon.stub().callsFake((_opts, perProjectCb) => { + perProjectCb("root.project", {getReader: () => ({fakeReader: true})}); + return Promise.resolve(["root.project"]); + }), + }; + + // Capture the buildServerInterface so the test can call getReaderForProject + // directly. The mocked BuildReader receives it on construction. + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + + const BuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + + await clock.tickAsync(10); + // Wait for VALIDATING to land. + await drainUntil(clock, statusEvents, (e) => e.status === "serve-validating"); + const buildCallsBeforeRequest = projectBuilder.build.callCount; + + // Issue a reader request for the validating project via the buildServerInterface. + const readerPromise = capturedInterface.getReaderForProject("library.x"); + + // Let any spurious queue tick attempt to land before we release validation. + await clock.tickAsync(20); + + // No second build cycle should have been kicked off while validating. + t.is(projectBuilder.build.callCount, buildCallsBeforeRequest, + "No additional build started while reader request was waiting on validation"); + + // Now release validation; it should resolve the reader and land on READY. + releaseValidation(); + const reader = await readerPromise; + t.deepEqual(reader, {fakeReader: true}, "Reader request resolved via validation's setReader"); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-ready"); + + const seq = statusEvents.map((e) => e.status); + const idxValidating = seq.indexOf("serve-validating"); + const idxReady = seq.lastIndexOf("serve-ready"); + // Crucial: no BUILDING between VALIDATING and the terminal READY. + t.is(seq.slice(idxValidating, idxReady).indexOf("serve-building"), -1, + `No serve-building between serve-validating and final serve-ready; got: ${seq.join(", ")}`); + }); + +// When validation finds a project's cache stale and a reader request was queued +// for that project during VALIDATING, the validation pass must enqueue a build +// for it before settling — otherwise the queued reader request is orphaned. +test.serial( + "serve-status: reader request during VALIDATING is built when validation finds cache stale", + async (t) => { + const {sinon, clock} = t.context; + + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* () { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + + let releaseValidation; + const validationGate = new Promise((resolve) => { + releaseValidation = resolve; + }); + + let buildCallCount = 0; + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().callsFake(async ({willValidate}, callback) => { + await willValidate?.("library.x"); + await validationGate; + // usesCache=false: cache is stale, project stays INITIAL. + await callback("library.x", { + getReader: () => ({fakeReader: true}), + }, {}, false); + }), + build: sinon.stub().callsFake((opts, perProjectCb) => { + buildCallCount++; + if (opts.includeRootProject) { + perProjectCb("root.project", {getReader: () => ({fakeReader: true})}); + } + for (const depName of opts.includedDependencies || []) { + perProjectCb(depName, {getReader: () => ({builtReader: depName})}); + } + return Promise.resolve( + (opts.includeRootProject ? ["root.project"] : []).concat(opts.includedDependencies || [])); + }), + }; + + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + + const BuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-validating"); + const buildCallsBeforeRequest = buildCallCount; + + // Queue a reader request for the validating project. + const readerPromise = capturedInterface.getReaderForProject("library.x"); + + // Release validation: cache is stale, so the validation pass settles without + // resolving the reader. The finally clause must enqueue a build. + releaseValidation(); + // Drain microtasks + the queue's 10 ms debounce so the follow-up build can run. + await clock.tickAsync(30); + + const readerResult = await readerPromise; + t.deepEqual(readerResult, {builtReader: "library.x"}, + "Reader request resolved via follow-up build"); + t.true(buildCallCount > buildCallsBeforeRequest, + "A follow-up build was triggered to satisfy the pending reader request"); + }); + +// When a source change lands during a validation pass, the server must transition +// VALIDATING → STALE right away rather than waiting for the pass to finish. +test.serial( + "serve-status: source change during VALIDATING transitions to STALE eagerly", async (t) => { + const {BuildServer, sinon, clock} = t.context; + + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* () { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + + let releaseValidation; + const validationGate = new Promise((resolve) => { + releaseValidation = resolve; + }); + + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().callsFake(async ({willValidate}, callback) => { + await willValidate?.("library.x"); + await validationGate; + await callback("library.x", {getReader: () => ({fakeReader: true})}, {}, true); + }), + build: sinon.stub().callsFake((_opts, perProjectCb) => { + perProjectCb("root.project", {getReader: () => ({fakeReader: true})}); + return Promise.resolve(["root.project"]); + }), + }; + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-validating"); + const validatingIdx = statusEvents.findIndex((e) => e.status === "serve-validating"); + + // Source change on a project currently in the validation set. This invalidates + // the project (firing its per-project abort signal which the validation pass is + // composing with) AND triggers the new VALIDATING → STALE branch in + // _projectResourceChanged. + buildServer._projectResourceChanged(libProject, "/x.js", false); + + // STALE must land synchronously — before the validation pass is even released. + const seqMid = statusEvents.map((e) => e.status); + const staleIdx = seqMid.indexOf("serve-stale", validatingIdx); + t.true(staleIdx > validatingIdx, + `Expected serve-stale promptly after the change; got: ${seqMid.join(", ")}`); + + // Release validation so the test can tear down cleanly. + releaseValidation(); + await clock.tickAsync(20); + }); + + diff --git a/packages/project/test/lib/build/ProjectBuilder.js b/packages/project/test/lib/build/ProjectBuilder.js index 1946b1d4f38..c401a2f60c2 100644 --- a/packages/project/test/lib/build/ProjectBuilder.js +++ b/packages/project/test/lib/build/ProjectBuilder.js @@ -775,3 +775,156 @@ test("_getElapsedTime", (t) => { const res = builder._getElapsedTime(process.hrtime()); t.truthy(res, "Returned a value"); }); + +test("validateCaches: initializes contexts via getRequiredProjectContexts and invokes callback per project", + async (t) => { + const {graph, taskRepository, ProjectBuilder, sinon} = t.context; + const builder = new ProjectBuilder({graph, taskRepository}); + + // Background validation must be able to initialize contexts for never-built projects + // itself, otherwise the post-(initial-)build pass over dependencies skipped by the + // initial build would find no contexts and silently no-op. + const validateCacheB = sinon.stub().resolves(true); + const validateCacheC = sinon.stub().resolves(false); + const ctxB = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache: validateCacheB, + getProject: sinon.stub().returns(getMockProject("library", "b")), + }; + const ctxC = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache: validateCacheC, + getProject: sinon.stub().returns(getMockProject("library", "c")), + }; + const getRequiredProjectContextsStub = sinon.stub(builder._buildContext, "getRequiredProjectContexts") + .resolves(new Map([ + ["project.b", ctxB], + ["project.c", ctxC], + ])); + + const callback = sinon.stub(); + const processed = await builder.validateCaches({ + projects: ["project.b", "project.c"], + }, callback); + + t.is(getRequiredProjectContextsStub.callCount, 1, + "getRequiredProjectContexts invoked once to initialize cold contexts"); + t.deepEqual(getRequiredProjectContextsStub.getCall(0).args[0], ["project.b", "project.c"], + "getRequiredProjectContexts called with the requested project names"); + t.deepEqual(processed, ["project.b", "project.c"], + "Returns the names of projects that were actually validated"); + t.is(validateCacheB.callCount, 1, "validateCache called for project.b"); + t.is(validateCacheC.callCount, 1, "validateCache called for project.c"); + t.is(callback.callCount, 2, "Callback invoked once per requested project"); + t.is(callback.getCall(0).args[0], "project.b", + "Callback invoked with project.b first (dependency-first order)"); + t.is(callback.getCall(0).args[3], true, "project.b reports usesCache=true"); + t.is(callback.getCall(1).args[0], "project.c"); + t.is(callback.getCall(1).args[3], false, "project.c reports usesCache=false"); + }); + +test("validateCaches: aborts on signal", async (t) => { + const {graph, taskRepository, ProjectBuilder, sinon} = t.context; + const builder = new ProjectBuilder({graph, taskRepository}); + + const controller = new AbortController(); + const validateCacheB = sinon.stub().callsFake(() => { + // Abort right after the first validation runs so the loop throws before reaching C + controller.abort(Object.assign(new Error("aborted"), {name: "AbortError"})); + return true; + }); + const validateCacheC = sinon.stub().resolves(true); + const ctxB = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache: validateCacheB, + getProject: sinon.stub().returns(getMockProject("library", "b")), + }; + const ctxC = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache: validateCacheC, + getProject: sinon.stub().returns(getMockProject("library", "c")), + }; + sinon.stub(builder._buildContext, "getRequiredProjectContexts").resolves(new Map([ + ["project.b", ctxB], + ["project.c", ctxC], + ])); + + await t.throwsAsync(builder.validateCaches({ + projects: ["project.b", "project.c"], + signal: controller.signal, + }, sinon.stub())); + + t.is(validateCacheB.callCount, 1, "project.b was validated before abort"); + t.is(validateCacheC.callCount, 0, "project.c was not validated after abort"); +}); + +test("validateCaches: rejects re-entry while a build is running", async (t) => { + const {graph, taskRepository, ProjectBuilder, sinon} = t.context; + const builder = new ProjectBuilder({graph, taskRepository}); + + // Simulate an in-flight build by manually flipping the private flag via a build call. + // Easiest: kick off two validateCaches in parallel and assert the second rejects. + let resolveFirst; + const firstValidatePromise = new Promise((resolve) => { + resolveFirst = resolve; + }); + const validateCache = sinon.stub().returns(firstValidatePromise); + const ctx = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache, + getProject: sinon.stub().returns(getMockProject("library", "b")), + }; + sinon.stub(builder._buildContext, "getRequiredProjectContexts").resolves(new Map([ + ["project.b", ctx], + ])); + + const first = builder.validateCaches({projects: ["project.b"]}); + const err = await t.throwsAsync(builder.validateCaches({projects: ["project.b"]})); + t.is(err.message, "A build is already running", + "Second concurrent call rejects with the same error as concurrent builds"); + + resolveFirst(true); + await first; +}); + +test("validateCaches: willValidate fires before each project's validateCache call", async (t) => { + const {graph, taskRepository, ProjectBuilder, sinon} = t.context; + const builder = new ProjectBuilder({graph, taskRepository}); + + const events = []; + const ctxB = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache: sinon.stub().callsFake(() => { + events.push("validate:b"); + return true; + }), + getProject: sinon.stub().returns(getMockProject("library", "b")), + }; + const ctxC = { + possiblyRequiresBuild: sinon.stub().returns(true), + validateCache: sinon.stub().callsFake(() => { + events.push("validate:c"); + return true; + }), + getProject: sinon.stub().returns(getMockProject("library", "c")), + }; + sinon.stub(builder._buildContext, "getRequiredProjectContexts").resolves(new Map([ + ["project.b", ctxB], + ["project.c", ctxC], + ])); + + await builder.validateCaches({ + projects: ["project.b", "project.c"], + willValidate: (name) => { + events.push(`will:${name}`); + }, + }, () => { + events.push("validated"); + }); + + t.deepEqual(events, [ + "will:project.b", "validate:b", "validated", + "will:project.c", "validate:c", "validated", + ], "willValidate fires before validateCache, callback after"); +}); + diff --git a/packages/project/test/lib/build/cache/ProjectBuildCache.js b/packages/project/test/lib/build/cache/ProjectBuildCache.js index de425c91740..e09267db91b 100644 --- a/packages/project/test/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/test/lib/build/cache/ProjectBuildCache.js @@ -1,6 +1,7 @@ import test from "ava"; import sinon from "sinon"; import ProjectBuildCache from "../../../../lib/build/cache/ProjectBuildCache.js"; +import Cache from "../../../../lib/build/cache/Cache.js"; // Helper to create mock Project instances function createMockProject(name = "test.project", id = "test-project-id") { @@ -1663,6 +1664,7 @@ test("restoreFrozenSources: cache hit creates CAS reader", async (t) => { async function createCacheInRestoringState({ resources = [createMockResource("/test.js", "hash1", 1000, 100, 1)], tasks = [["task1", false]], + cacheMode, } = {}) { const project = createMockProject(); const cacheManager = createMockCacheManager(); @@ -1719,7 +1721,7 @@ async function createCacheInRestoringState({ cacheManager.readIndexCache.returns(indexCache); - const cache = await ProjectBuildCache.create(project, buildSignature, cacheManager); + const cache = await ProjectBuildCache.create(project, buildSignature, cacheManager, cacheMode); await cache.initSourceIndex(); // Spy on _refreshDependencyIndices so we can verify whether it's called @@ -1800,3 +1802,89 @@ test("prepareProjectBuildAndValidateCache: subsequent dependency changes go thro "_refreshDependencyIndices should NOT be called for REQUIRES_UPDATE state " + "(#flushPendingChanges handles it instead)"); }); + +// ===== validateCache TESTS ===== + +test("validateCache: returns false for empty cache (INITIAL state)", async (t) => { + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + const cache = await ProjectBuildCache.create(project, "sig", cacheManager); + await cache.initSourceIndex(); + + const mockDependencyReader = { + byGlob: sinon.stub().resolves([]), + byPath: sinon.stub().resolves(null) + }; + + const result = await cache.validateCache(mockDependencyReader); + + t.is(result, false, "Returns false for empty cache"); +}); + +test("validateCache: Cache.Off short-circuits and returns false", async (t) => { + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + const cache = await ProjectBuildCache.create(project, "sig", cacheManager, Cache.Off); + + const mockDependencyReader = { + byGlob: sinon.stub().resolves([]), + byPath: sinon.stub().resolves(null) + }; + + const result = await cache.validateCache(mockDependencyReader); + + t.is(result, false, "Returns false in Cache.Off mode"); + t.false(cache.isFresh(), "Cache is not fresh in Cache.Off mode"); +}); + +test("validateCache: warm cache without changes is fresh", async (t) => { + const {cache, refreshSpy, mockDependencyReader} = await createCacheInRestoringState(); + + const result = await cache.validateCache(mockDependencyReader); + + t.false(refreshSpy.called, "_refreshDependencyIndices is skipped without dependency changes"); + // With no persisted result metadata available, validateCache returns false (NO_CACHE). + t.is(result, false, "validateCache returns false when no result metadata is cached"); +}); + +test("validateCache: prepareProjectBuildAndValidateCache delegates to validateCache", async (t) => { + const {cache, mockDependencyReader} = await createCacheInRestoringState(); + + // Spy on validateCache to confirm prepareProjectBuildAndValidateCache delegates + const validateSpy = sinon.spy(cache, "validateCache"); + + const prepResult = await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + + t.true(validateSpy.calledOnce, "validateCache is invoked exactly once by prepareProjectBuildAndValidateCache"); + t.is(validateSpy.firstCall.args[0], mockDependencyReader, + "dependencyReader is forwarded to validateCache"); + t.is(prepResult, await validateSpy.firstCall.returnValue, + "prepareProjectBuildAndValidateCache returns whatever validateCache returned"); +}); + +test("validateCache: Cache.Force throws when source changes are detected", async (t) => { + // Build a warm cache pinned to Force whose initSourceIndex succeeds (no source changes + // at init time), then introduce a change before invoking validateCache so the + // REQUIRES_UPDATE branch in validateCache triggers the Force-mode throw. + const {cache: forceCache, project, mockDependencyReader} = await createCacheInRestoringState({ + cacheMode: Cache.Force, + }); + + // Simulate an in-build source change: swap the resource with one whose integrity differs + // from the indexed value, so #updateSourceIndex detects an "updated" path. + const newResource = createMockResource("/test.js", "hash2", 2000, 200, 1); + project.getSourceReader.callsFake(() => ({ + byGlob: sinon.stub().resolves([newResource]), + byPath: sinon.stub().callsFake((path) => { + return Promise.resolve(path === "/test.js" ? newResource : null); + }) + })); + forceCache.projectSourcesChanged(["/test.js"]); + + const err = await t.throwsAsync( + () => forceCache.validateCache(mockDependencyReader), + undefined, + "validateCache throws when Force mode detects source changes" + ); + t.regex(err.message, /Force.*mode.*stale/i, "Error message mentions Force mode and stale cache"); +}); From ec3d537ee49f4b5d2ef48b78a0d4b3d618a43a57 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 6 Jul 2026 16:05:23 +0200 Subject: [PATCH 2/8] refactor(logger): Extract shared helpers for new build/validating states Two dedup passes on the logger code that grew with the VALIDATING state. `Serve.#emitStatus(status, payload, fallbackMessage, {level})` replaces the 4-line pattern each status method (`ready`, `stale`, `building`, `validating`, `buildDone`, `serveError`) repeated: build the `ui5.serve-status` payload, emit it, and if no listener responded, log at the same level. Public method names, argument shapes, event payload keys and fallback messages are unchanged. New statuses land as two-line wrappers. `SPINNING_STATES` moves onto `state/build.js`. Both `InteractiveConsole.#scheduleTick` and `renderStatusLine` gated on `state === BUILDING || state === VALIDATING`; a third animated state meant matching changes at each site. The tick scheduler now consults the set. The renderer keeps its per-state cases because each spinning state renders distinct colour, label and payload; only the tick gate benefits from a shared predicate today. --- packages/logger/lib/loggers/Serve.js | 69 ++++++------------- .../logger/lib/writers/InteractiveConsole.js | 5 +- .../writers/interactiveConsole/state/build.js | 5 ++ 3 files changed, 28 insertions(+), 51 deletions(-) diff --git a/packages/logger/lib/loggers/Serve.js b/packages/logger/lib/loggers/Serve.js index 9f5c8f3b6de..2146365ddc0 100644 --- a/packages/logger/lib/loggers/Serve.js +++ b/packages/logger/lib/loggers/Serve.js @@ -19,30 +19,31 @@ import Logger from "./Logger.js"; class Serve extends Logger { static SERVE_STATUS_EVENT_NAME = "ui5.serve-status"; - ready() { - const level = "info"; + // Emit a `ui5.serve-status` event; if no listener is attached, fall back to + // a plain log line at the same level. All public status methods route + // through here so the event payload shape (`{level, status, ...}`) and the + // no-listener fallback stay in sync across states. + #emitStatus(status, payload, fallbackMessage, {level = "info"} = {}) { const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { level, - status: "serve-ready", + status, + ...payload, }); if (!hasListeners) { - this._log(level, `Server ready`); + this._log(level, fallbackMessage); } } + ready() { + this.#emitStatus("serve-ready", {}, `Server ready`); + } + stale(changedProjects) { if (!changedProjects || !Array.isArray(changedProjects)) { throw new Error("loggers/Serve#stale: Missing or incorrect changedProjects parameter"); } - const level = "info"; - const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { - level, - status: "serve-stale", - changedProjects, - }); - if (!hasListeners) { - this._log(level, `Sources changed in: ${changedProjects.join(", ")}`); - } + this.#emitStatus("serve-stale", {changedProjects}, + `Sources changed in: ${changedProjects.join(", ")}`); } validating(validatingProjects) { @@ -50,26 +51,12 @@ class Serve extends Logger { throw new Error( "loggers/Serve#validating: Missing or incorrect validatingProjects parameter"); } - const level = "info"; - const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { - level, - status: "serve-validating", - validatingProjects, - }); - if (!hasListeners) { - this._log(level, `Validating caches for: ${validatingProjects.join(", ")}`); - } + this.#emitStatus("serve-validating", {validatingProjects}, + `Validating caches for: ${validatingProjects.join(", ")}`); } building() { - const level = "info"; - const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { - level, - status: "serve-building", - }); - if (!hasListeners) { - this._log(level, `Building...`); - } + this.#emitStatus("serve-building", {}, `Building...`); } buildDone(hrtime) { @@ -77,15 +64,8 @@ class Serve extends Logger { typeof hrtime[0] !== "number" || typeof hrtime[1] !== "number") { throw new Error("loggers/Serve#buildDone: Missing or incorrect hrtime parameter"); } - const level = "info"; - const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { - level, - status: "serve-build-done", - hrtime, - }); - if (!hasListeners) { - this._log(level, `Build finished in ${prettyHrtime(hrtime)}`); - } + this.#emitStatus("serve-build-done", {hrtime}, + `Build finished in ${prettyHrtime(hrtime)}`); } // Named serveError rather than error to avoid shadowing Logger.prototype.error @@ -93,15 +73,8 @@ class Serve extends Logger { if (!error) { throw new Error("loggers/Serve#serveError: Missing error parameter"); } - const level = "error"; - const hasListeners = this._emit(Serve.SERVE_STATUS_EVENT_NAME, { - level, - status: "serve-error", - error, - }); - if (!hasListeners) { - this._log(level, `Server error: ${error.message || error}`); - } + this.#emitStatus("serve-error", {error}, + `Server error: ${error.message || error}`, {level: "error"}); } } diff --git a/packages/logger/lib/writers/InteractiveConsole.js b/packages/logger/lib/writers/InteractiveConsole.js index 04b37db57f3..d7d6441b5d0 100644 --- a/packages/logger/lib/writers/InteractiveConsole.js +++ b/packages/logger/lib/writers/InteractiveConsole.js @@ -9,7 +9,7 @@ import {createProjectState, setProject, enableProjectPlaceholders} from "./inter import {createServerState, setListening, enableServerPlaceholders} from "./interactiveConsole/state/server.js"; import { createBuildState, beginBuild, advanceToProject, setTask, transitionTo, setError, STATES, - enableBuildPlaceholders, + SPINNING_STATES, enableBuildPlaceholders, } from "./interactiveConsole/state/build.js"; import { renderHeaderRegion, renderProjectRegion, renderServerRegion, renderBuildRegion, @@ -403,8 +403,7 @@ class InteractiveConsole { if (this.#stopped) { return; } - if (this.#buildState.state !== STATES.BUILDING && - this.#buildState.state !== STATES.VALIDATING) { + if (!SPINNING_STATES.has(this.#buildState.state)) { return; } this.#tickTimer = setInterval(() => { diff --git a/packages/logger/lib/writers/interactiveConsole/state/build.js b/packages/logger/lib/writers/interactiveConsole/state/build.js index a620fb88145..182b5e7cfc3 100644 --- a/packages/logger/lib/writers/interactiveConsole/state/build.js +++ b/packages/logger/lib/writers/interactiveConsole/state/build.js @@ -11,6 +11,11 @@ export const STATES = Object.freeze({ ERROR: "error", }); +// States that animate a spinner. Consulted by both the tick scheduler in the +// interactive console writer and the status-line renderer, so a state either +// spins in both places or in neither. +export const SPINNING_STATES = new Set([STATES.BUILDING, STATES.VALIDATING]); + export function createBuildState() { return { state: STATES.INITIAL, From 22bc9a5e9499b3c7d4c6947d30e33fd787c44efd Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Fri, 3 Jul 2026 17:42:51 +0200 Subject: [PATCH 3/8] test(project): Factor BuildServer serve-status test scaffolding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every serve-status test open-coded the same listener wiring (`statusEvents.push` on `ui5.serve-status`, teardown), and the two VALIDATING end-state tests re-declared the same root/lib graph literal. Add two helpers: - `makeStatusRecorder(t)` — accumulating event array plus teardown. - `makeGraphWithLib()` — graph with one library dependency, the shape needed to leave one INITIAL project after a build cycle (the trigger for VALIDATING). No runtime impact; the 16 tests continue to pass. --- .../project/test/lib/build/BuildServer.js | 105 +++++++----------- 1 file changed, 41 insertions(+), 64 deletions(-) diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 576c7b26923..644ce31c6a0 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -19,6 +19,36 @@ async function drainUntil(clock, statusEvents, predicate, {maxTicks = 100} = {}) `events: ${statusEvents.map((e) => e.status).join(", ")}`); } +// Graph with a single library dependency behind the root project. Used by the +// BUILDING → VALIDATING → … tests to leave one INITIAL project after the build +// cycle, which is the trigger for the post-build validation pass. +function makeGraphWithLib() { + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.x"}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.x"], + getProject: (name) => name === "root.project" ? rootProject : libProject, + traverseDependents: function* () { + yield {project: rootProject}; + yield {project: libProject}; + }, + }; + return {rootProject, libProject, graph}; +} + +// Attach a `ui5.serve-status` listener to the current process, register a +// teardown to detach it, and return the accumulating event array. Every +// serve-status test needs exactly this shape. +function makeStatusRecorder(t) { + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + return statusEvents; +} + test.beforeEach(async (t) => { const sinon = t.context.sinon = sinonGlobal.createSandbox(); t.context.clock = sinon.useFakeTimers(); @@ -187,10 +217,7 @@ test.serial("serve-status: serve-stale emitted on debounced sources change", (t) // exact ordering and counts. async function runInitialBuildCycle(t, {buildResult = "ok"} = {}) { const {BuildServer, graph, projectBuilder, sinon, clock} = t.context; - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); let buildResolve; let buildReject; @@ -241,10 +268,7 @@ test.serial("serve-status: initial build cycle emits building → buildDone → test.serial( "serve-status: source change mid-build emits exactly one stale at cycle end", async (t) => { const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); let buildResolve; let perProjectCb; @@ -282,10 +306,7 @@ test.serial( test.serial("serve-status: build failure emits serveError and no orphan building", async (t) => { const {BuildServer, graph, projectBuilder, sinon, clock} = t.context; - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildError = new Error("Build blew up"); projectBuilder.build = sinon.stub().rejects(buildError); @@ -319,18 +340,7 @@ test.serial( // Augment the graph with a single library dependency so the build cycle leaves // one INITIAL project behind, which is exactly the trigger for VALIDATING. - const rootProject = {getName: () => "root.project"}; - const libProject = {getName: () => "library.x"}; - const graph = { - getRoot: () => rootProject, - getProjects: () => [rootProject, libProject], - getTransitiveDependencies: () => ["library.x"], - getProject: (name) => name === "root.project" ? rootProject : libProject, - traverseDependents: function* (_projectName) { - yield {project: rootProject}; - yield {project: libProject}; - }, - }; + const {graph} = makeGraphWithLib(); // validateCaches: report library.x's cache as fresh — usesCache=true triggers the // promote-to-FRESH path that closes out the validation pass with no stale projects. @@ -349,10 +359,7 @@ test.serial( }), }; - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); t.teardown(() => buildServer.destroy()); @@ -386,18 +393,7 @@ test.serial( "serve-status: VALIDATING → STALE when validation finds a stale cache", async (t) => { const {BuildServer, sinon, clock} = t.context; - const rootProject = {getName: () => "root.project"}; - const libProject = {getName: () => "library.x"}; - const graph = { - getRoot: () => rootProject, - getProjects: () => [rootProject, libProject], - getTransitiveDependencies: () => ["library.x"], - getProject: (name) => name === "root.project" ? rootProject : libProject, - traverseDependents: function* () { - yield {project: rootProject}; - yield {project: libProject}; - }, - }; + const {graph} = makeGraphWithLib(); // usesCache=false → project must stay INITIAL → final state is STALE. const projectBuilder = { @@ -415,10 +411,7 @@ test.serial( }), }; - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); t.teardown(() => buildServer.destroy()); @@ -465,11 +458,7 @@ test.serial( return Promise.resolve(["root.project"]); }), }; - - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); t.teardown(() => buildServer.destroy()); @@ -563,11 +552,7 @@ test.serial( "../../../lib/build/BuildReader.js": CapturingBuildReader, "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, })).default; - - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); t.teardown(() => buildServer.destroy()); @@ -671,11 +656,7 @@ test.serial( "../../../lib/build/BuildReader.js": CapturingBuildReader, "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, })).default; - - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); t.teardown(() => buildServer.destroy()); @@ -737,11 +718,7 @@ test.serial( return Promise.resolve(["root.project"]); }), }; - - const statusEvents = []; - const statusHandler = (evt) => statusEvents.push(evt); - process.on("ui5.serve-status", statusHandler); - t.teardown(() => process.off("ui5.serve-status", statusHandler)); + const statusEvents = makeStatusRecorder(t); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); t.teardown(() => buildServer.destroy()); From 4e6754f0b5f87ab1cff48536b8512bc7dd378e38 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 6 Jul 2026 16:06:24 +0200 Subject: [PATCH 4/8] refactor(project): Deduplicate ProjectBuilder #build and #validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both loops open-coded the same three-step setup: call `getRequiredProjectContexts(requestedProjects)`, traverse with `traverseDependenciesDepthFirst(true)`, and materialize a DFS-ordered queue with a parallel `processedProjectNames` array. They then iterated with `while (queue.length) { queue.shift() }`, O(n^2) in the queue length for no reason. Extract `#buildProjectQueue(requestedProjects)` returning `{projectBuildContexts, queue, processedProjectNames}`; the `getRequiredProjectContexts` perf-log line moves inside. Both callers start with a one-line destructure and keep their differing pre/post work inline. Both loops switch to `for (const projectBuildContext of queue)` — the queue is built once from the pre-computed traversal and is not mutated during iteration. The per-project loop bodies stay in each caller: their contents differ (build vs. validate step sequences), and the shared shape (`possiblyRequiresBuild` gate, abort classification) is short enough that closure-passing wouldn't earn its keep. --- packages/project/lib/build/ProjectBuilder.js | 88 ++++++++++---------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index 9fa62fd95b3..a2161a710a7 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -339,6 +339,46 @@ class ProjectBuilder { return requestedProjects; } + /** + * Resolves the project build contexts for the requested projects (plus their transitive + * build-time dependencies) and returns them in dependency-first (DFS) order. Shared setup + * for {@link #build} and {@link #validate}; the DFS ordering ensures that dependencies are + * processed before their dependents, so propagated resource changes reach dependents in + * time. + * + * @param {string[]} requestedProjects Names of the projects the caller wants to process + * @returns {Promise<{ + * projectBuildContexts: Map, + * queue: @ui5/project/build/helpers/ProjectBuildContext[], + * processedProjectNames: string[] + * }>} + * The full context map, plus a DFS-ordered array of the contexts that will be walked + * and the matching project names in the same order. + */ + async #buildProjectQueue(requestedProjects) { + const reqStart = performance.now(); + const projectBuildContexts = await this._buildContext.getRequiredProjectContexts(requestedProjects); + if (this.#log.isLevelEnabled("perf")) { + this.#log.perf( + `getRequiredProjectContexts completed in ${(performance.now() - reqStart).toFixed(2)} ms`); + } + + const queue = []; + const processedProjectNames = []; + for (const {project} of this._graph.traverseDependenciesDepthFirst(true)) { + const projectName = project.getName(); + const projectBuildContext = projectBuildContexts.get(projectName); + if (projectBuildContext) { + // Build context exists + // => This project needs to be built or, in case it has already + // been built, it's build result needs to be written out (if requested) + queue.push(projectBuildContext); + processedProjectNames.push(projectName); + } + } + return {projectBuildContexts, queue, processedProjectNames}; + } + /** * Internal build implementation that orchestrates the actual build process * @@ -358,27 +398,7 @@ class ProjectBuilder { try { cleanupSigHooks = this._registerCleanupSigHooks(); this.#log.info(`Preparing build for projects: ${requestedProjects.join(", ")}`); - const reqStart = performance.now(); - const projectBuildContexts = await this._buildContext.getRequiredProjectContexts(requestedProjects); - if (this.#log.isLevelEnabled("perf")) { - this.#log.perf( - `getRequiredProjectContexts completed in ${(performance.now() - reqStart).toFixed(2)} ms`); - } - - // Create build queue based on graph depth-first search to ensure correct build order - const queue = []; - const processedProjectNames = []; - for (const {project} of this._graph.traverseDependenciesDepthFirst(true)) { - const projectName = project.getName(); - const projectBuildContext = projectBuildContexts.get(projectName); - if (projectBuildContext) { - // Build context exists - // => This project needs to be built or, in case it has already - // been built, it's build result needs to be written out (if requested) - queue.push(projectBuildContext); - processedProjectNames.push(projectName); - } - } + const {queue, processedProjectNames} = await this.#buildProjectQueue(requestedProjects); this.#log.setProjects(queue.map((projectBuildContext) => { return projectBuildContext.getProject().getName(); @@ -394,8 +414,7 @@ class ProjectBuilder { const startTime = process.hrtime(); try { - while (queue.length) { - const projectBuildContext = queue.shift(); + for (const projectBuildContext of queue) { const project = projectBuildContext.getProject(); const projectName = project.getName(); const projectType = project.getType(); @@ -492,9 +511,9 @@ class ProjectBuilder { // build flow through `projectSourcesChanged` → `#flushPendingChanges` → // `#updateSourceIndex` at the next build's start, leaving the source index // consistent by the time `#revalidateSourceIndex` runs at build end. - const projectBuildContexts = - await this._buildContext.getRequiredProjectContexts(requestedProjects); - if (projectBuildContexts.size === 0) { + const {projectBuildContexts, queue, processedProjectNames} = + await this.#buildProjectQueue(requestedProjects); + if (queue.length === 0) { this.#log.verbose(`No projects to validate`); return []; } @@ -505,24 +524,9 @@ class ProjectBuilder { // checking membership per project is O(N) on an array — hoist to a Set. const requestedSet = new Set(requestedProjects); - // Build queue based on graph depth-first search to ensure that dependencies - // are validated before their dependents, so propagated resource changes reach - // dependents before those dependents validate themselves. - const queue = []; - const processedProjectNames = []; - for (const {project} of this._graph.traverseDependenciesDepthFirst(true)) { - const projectName = project.getName(); - const projectBuildContext = projectBuildContexts.get(projectName); - if (projectBuildContext) { - queue.push(projectBuildContext); - processedProjectNames.push(projectName); - } - } - try { - while (queue.length) { + for (const projectBuildContext of queue) { signal?.throwIfAborted(); - const projectBuildContext = queue.shift(); const project = projectBuildContext.getProject(); const projectName = project.getName(); const isRequested = requestedSet.has(projectName); From 8e00d924471f0861c84f8cad3d3c8698e0f321ba Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Sun, 5 Jul 2026 09:17:20 +0200 Subject: [PATCH 5/8] refactor(project): Collapse the two cache-validation entry points `ProjectBuildContext.prepareProjectBuildAndValidateCache` and `ProjectBuildContext.validateCache` routed through the same `#runCacheValidation` helper, differing only in the underlying cache call and the perf-log label. `ProjectBuildCache` mirrored the split with a delegator that captured readers, discarded pending stage-cache entries, and called `validateCache`. Collapse to a single `validateCache(depReader?, {prepareForBuild})`, the flag gating pre-build side effects (`stageCache.discardPending()` + reader capture). `ProjectBuildContext.#runCacheValidation` goes away; both `ProjectBuilder` call sites log under `validateCache`. Tests and skill docs mirror the API change. --- .../skills/incremental-build/architecture.md | 4 +- .../performance-investigation.md | 8 +-- packages/project/lib/build/ProjectBuilder.js | 4 +- .../lib/build/cache/ProjectBuildCache.js | 52 ++++++--------- .../lib/build/helpers/ProjectBuildContext.js | 53 ++++----------- .../test/lib/build/BuildServer.integration.js | 4 +- .../project/test/lib/build/ProjectBuilder.js | 14 ++-- .../test/lib/build/cache/ProjectBuildCache.js | 65 +++++++------------ 8 files changed, 76 insertions(+), 128 deletions(-) diff --git a/.claude/skills/incremental-build/architecture.md b/.claude/skills/incremental-build/architecture.md index c56c18cf04e..54531e5776d 100644 --- a/.claude/skills/incremental-build/architecture.md +++ b/.claude/skills/incremental-build/architecture.md @@ -147,7 +147,7 @@ At build completion, `#revalidateSourceIndex()` re-reads all source files and co -> Create fresh ResourceIndex from all source resources -> #combinedIndexState = INITIAL -prepareProjectBuildAndValidateCache() +validateCache({prepareForBuild: true}) -> State is INITIAL -> return false (no cache to validate) For each task: @@ -178,7 +178,7 @@ projectSourcesChanged(changedPaths) / dependencyResourcesChanged(changedPaths) -> Records changed paths -> Sets #combinedIndexState = REQUIRES_UPDATE -prepareProjectBuildAndValidateCache() +validateCache({prepareForBuild: true}) -> State is REQUIRES_UPDATE -> #flushPendingChanges(): #updateSourceIndex(changedPaths) -> reads resources from source reader diff --git a/.claude/skills/incremental-build/performance-investigation.md b/.claude/skills/incremental-build/performance-investigation.md index 9f5f1890b7d..4b122e8dde1 100644 --- a/.claude/skills/incremental-build/performance-investigation.md +++ b/.claude/skills/incremental-build/performance-investigation.md @@ -134,10 +134,10 @@ perf Validated result cache for project sap.ui.core in 13 ms After validation: ``` -perf prepareProjectBuildAndValidateCache for sap.ui.core completed in 34 ms (usesCache=true) +perf validateCache for sap.ui.core completed in 34 ms (usesCache=true) info ✔ Skipping build of library project sap.ui.core OR -perf prepareProjectBuildAndValidateCache for sap.m completed in 0.48 ms (usesCache=false) +perf validateCache for sap.m completed in 0.48 ms (usesCache=false) info ❯ Building library project sap.m... ``` @@ -238,9 +238,9 @@ All `log.perf()` statements by source file: | `BuildContext.js` | `Parallel source index initialization completed` | Total time for parallel `initSourceIndex` across all projects | | `BuildContext.js` | `getRequiredProjectContexts completed` | Discovery + index init for all required projects | | `ProjectBuildContext.js` | `getDependenciesReader completed` | Creating the dependency reader for a project | -| `ProjectBuildContext.js` | `ProjectBuildCache.prepareProjectBuildAndValidateCache completed` | Full cache validation for one project | +| `ProjectBuildContext.js` | `ProjectBuildCache.validateCache completed` | Full cache validation for one project | | `ProjectBuilder.js` | `getRequiredProjectContexts completed` | Top-level timing (includes context creation) | -| `ProjectBuilder.js` | `prepareProjectBuildAndValidateCache for {project} completed` | Per-project validation with `usesCache` flag | +| `ProjectBuilder.js` | `validateCache for {project} completed` | Per-project validation with `usesCache` flag | | `TaskRunner.js` | `Task {name} finished in {N} ms` | Individual task execution time | | `ProjectBuildCache.js` | `#initSourceIndex fromCacheWithDelta` | Delta detection: resource count, changed count | | `ProjectBuildCache.js` | `Initialized source index for project` | Total source index init (glob + cache read + delta) | diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index a2161a710a7..4d5792d87d9 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -425,10 +425,10 @@ class ProjectBuilder { this.#log.skipProjectBuild(projectName, projectType); } else { const prepStart = performance.now(); - const usesCache = await projectBuildContext.prepareProjectBuildAndValidateCache(); + const usesCache = await projectBuildContext.validateCache({prepareForBuild: true}); if (this.#log.isLevelEnabled("perf")) { this.#log.perf( - `prepareProjectBuildAndValidateCache for ${projectName} ` + + `validateCache for ${projectName} ` + `completed in ${(performance.now() - prepStart).toFixed(2)} ms ` + `(usesCache=${usesCache})`); } diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index b3865c9b46a..30dd53d3268 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -150,45 +150,34 @@ export default class ProjectBuildCache { } /** - * Sets the dependency reader for accessing dependency resources - * - * The dependency reader is used by tasks to access resources from project - * dependencies. Must be set before tasks that require dependencies are executed. - * - * @public - * @param {@ui5/fs/AbstractReader} dependencyReader Reader for dependency resources - * @returns {Promise} - * Array of changed resource paths since last build, true if cache is fresh, false - * if cache is empty - */ - async prepareProjectBuildAndValidateCache(dependencyReader) { - // Discard any in-memory StageCache entries that a prior aborted build left in - // the queue. Successful builds flush the queue in writeCache, so this is a - // no-op in the common case. - this.#stageCache.discardPending(); - - this.#currentProjectReader = this.#project.getReader(); - - this.#currentDependencyReader = dependencyReader; - - return this.validateCache(dependencyReader); - } - - /** - * Validates the current build cache state without performing build-prep side effects. + * Validates the current build cache state. * * Flushes any pending source/dependency changes, refreshes dependency indices on first use, * and attempts to locate a cached result stage. Safe to call independently of a build attempt * (e.g. to check whether the cache is stale). * + * When `prepareForBuild` is true, additionally performs the side effects required before a + * project build: discards any in-memory StageCache entries left over from a prior aborted + * build (successful builds flush the queue in writeCache, so this is a no-op in the common + * case) and captures the current project and dependency readers for later use by + * recordTaskResult. + * * @public * @param {@ui5/fs/AbstractReader} dependencyReader Reader for dependency resources, used to * refresh dependency indices when required + * @param {object} [options] + * @param {boolean} [options.prepareForBuild=false] Run the pre-build side effects before + * validating (see method description) * @returns {Promise} * Array of changed resource paths since last build, true if cache is fresh, false * if cache is empty */ - async validateCache(dependencyReader) { + async validateCache(dependencyReader, {prepareForBuild = false} = {}) { + if (prepareForBuild) { + this.#stageCache.discardPending(); + this.#currentProjectReader = this.#project.getReader(); + this.#currentDependencyReader = dependencyReader; + } // When cache=Off, don't validate or use result cache if (this.#cacheMode === Cache.Off) { log.verbose(`Cache is in "Off" mode for project ${this.#project.getName()}. ` + @@ -1275,11 +1264,10 @@ export default class ProjectBuildCache { this.#combinedIndexState = INDEX_STATES.RESTORING_PROJECT_INDICES; this.#sourceIndex = null; this.#taskCache.clear(); - // Result cache state must also be reset: prepareProjectBuildAndValidateCache may have - // already transitioned it to NO_CACHE or FRESH_AND_IN_USE in the aborted build. The - // retry's prepareProjectBuildAndValidateCache asserts PENDING_VALIDATION after the - // dependency-index restore step, so a leftover non-PENDING_VALIDATION value would - // trip that assertion. + // Result cache state must also be reset: the aborted build's validateCache may + // have already transitioned it to NO_CACHE or FRESH_AND_IN_USE. The retry's + // validateCache asserts PENDING_VALIDATION after the dependency-index restore + // step, so a leftover non-PENDING_VALIDATION value would trip that assertion. this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION; throw new SourceChangedDuringBuildError(this.#project.getName()); diff --git a/packages/project/lib/build/helpers/ProjectBuildContext.js b/packages/project/lib/build/helpers/ProjectBuildContext.js index f73388525b8..f7cb3913135 100644 --- a/packages/project/lib/build/helpers/ProjectBuildContext.js +++ b/packages/project/lib/build/helpers/ProjectBuildContext.js @@ -244,7 +244,7 @@ class ProjectBuildContext { * Early check whether a project build is possibly required. * * In some cases, the cache state cannot be determined until all dependencies have been processed and - * the cache has been updated with that information. This happens during prepareProjectBuildAndValidateCache(). + * the cache has been updated with that information. This happens during validateCache(). * * This method allows for an early check whether a project build can be skipped. * @@ -260,46 +260,21 @@ class ProjectBuildContext { } /** - * Prepares the project build by updating and validating the build cache + * Validates the current build cache. * - * Creates a dependency reader and validates the cache state against current resources. - * Must be called before buildProject(). + * Fetches a fresh dependency reader, asks the build cache whether it is stale, and + * propagates any detected resource changes to dependents. When `prepareForBuild` is + * true, additionally runs the pre-build side effects on the underlying cache + * (discarding pending stage-cache entries from a prior aborted build and capturing + * the current project/dependency readers). * + * @param {object} [options] + * @param {boolean} [options.prepareForBuild=false] Run pre-build side effects on the + * underlying cache. Must be true when this call precedes {@link #buildProject}. * @returns {Promise} * True if a valid cache was found and is being used. False otherwise (indicating a build is required). */ - async prepareProjectBuildAndValidateCache() { - return this.#runCacheValidation( - "prepareProjectBuildAndValidateCache", - (cache, depReader) => cache.prepareProjectBuildAndValidateCache(depReader)); - } - - /** - * Validates the current build cache without performing build-prep side effects. - * - * Creates a dependency reader and asks the build cache whether it is stale, - * propagating any detected resource changes to dependents. - * - * @returns {Promise} - * True if a valid cache was found and is being used. False otherwise. - */ - async validateCache() { - return this.#runCacheValidation( - "validateCache", - (cache, depReader) => cache.validateCache(depReader)); - } - - /** - * Shared implementation for the two cache-validation entry points: fetches a fresh - * dependency reader, invokes the supplied cache call, and propagates any changed - * resource paths to dependents. - * - * @param {string} label Method name used for perf logging - * @param {Function} call Callback receiving (cache, depReader); returns boolean or string[] - * @returns {Promise} - * True if a valid cache was found and is being used. False otherwise. - */ - async #runCacheValidation(label, call) { + async validateCache({prepareForBuild = false} = {}) { const perfEnabled = this._log.isLevelEnabled("perf"); const readerStart = perfEnabled ? performance.now() : 0; const depReader = await this.getTaskRunner().getDependenciesReader( @@ -311,10 +286,10 @@ class ProjectBuildContext { `getDependenciesReader completed in ${(performance.now() - readerStart).toFixed(2)} ms`); } const cacheStart = perfEnabled ? performance.now() : 0; - const boolOrChangedPaths = await call(this.getBuildCache(), depReader); + const boolOrChangedPaths = await this.getBuildCache().validateCache(depReader, {prepareForBuild}); if (perfEnabled) { this._log.perf( - `ProjectBuildCache.${label} completed in ` + + `ProjectBuildCache.validateCache completed in ` + `${(performance.now() - cacheStart).toFixed(2)} ms`); } if (Array.isArray(boolOrChangedPaths)) { @@ -329,7 +304,7 @@ class ProjectBuildContext { * Builds the project by running all required tasks * * Executes all configured build tasks for the project using the task runner. - * Must be called after prepareProjectBuildAndValidateCache(). + * Must be called after validateCache({prepareForBuild: true}). * * @param {AbortSignal} [signal] Abort signal */ diff --git a/packages/project/test/lib/build/BuildServer.integration.js b/packages/project/test/lib/build/BuildServer.integration.js index fbd63f0e11f..0abcf7765a9 100644 --- a/packages/project/test/lib/build/BuildServer.integration.js +++ b/packages/project/test/lib/build/BuildServer.integration.js @@ -952,7 +952,7 @@ test.serial( } ); -// Regression: a build that hits the NO_CACHE state in prepareProjectBuildAndValidateCache +// Regression: a build that hits the NO_CACHE state in validateCache({prepareForBuild: true}) // (because the source signature does not match anything in the persistent cache) and then // throws SourceChangedDuringBuildError from allTasksCompleted used to fail on retry with // "Unexpected result cache state after restoring dependency indices for project XYZ: no_cache". @@ -964,7 +964,7 @@ test.serial( // reader request drives a second build. // 3. The second build's #initSourceIndex finds an existing index cache and transitions to // RESTORING_DEPENDENCY_INDICES (rather than INITIAL, which short-circuits prepare). -// 4. prepareProjectBuildAndValidateCache sees a source-signature mismatch against the +// 4. validateCache({prepareForBuild: true}) sees a source-signature mismatch against the // persisted result cache and sets #resultCacheState = NO_CACHE. // 5. A *further* on-disk source change lands during the second build, but the watcher path // is stubbed so the abort signal is never set. allTasksCompleted's revalidateSourceIndex diff --git a/packages/project/test/lib/build/ProjectBuilder.js b/packages/project/test/lib/build/ProjectBuilder.js index c401a2f60c2..d6360ee1305 100644 --- a/packages/project/test/lib/build/ProjectBuilder.js +++ b/packages/project/test/lib/build/ProjectBuilder.js @@ -113,12 +113,12 @@ test("build", async (t) => { const requiresBuildStub = sinon.stub().returns(true); const possiblyRequiresBuildStub = sinon.stub().returns(true); - const prepareProjectBuildAndValidateCacheStub = sinon.stub().resolves(false); + const validateCacheStub = sinon.stub().resolves(false); const buildProjectStub = sinon.stub().resolves(); const writeBuildCacheStub = sinon.stub().resolves(); const projectBuildContextMock = { possiblyRequiresBuild: possiblyRequiresBuildStub, - prepareProjectBuildAndValidateCache: prepareProjectBuildAndValidateCacheStub, + validateCache: validateCacheStub, buildProject: buildProjectStub, writeBuildCache: writeBuildCacheStub, requiresBuild: requiresBuildStub, @@ -226,11 +226,11 @@ test("build: Failure", async (t) => { sinon.stub(builder, "_createProjectFilter").returns(filterProjectStub); const possiblyRequiresBuildStub = sinon.stub().returns(true); - const prepareProjectBuildAndValidateCacheStub = sinon.stub().resolves(false); + const validateCacheStub = sinon.stub().resolves(false); const buildProjectStub = sinon.stub().rejects(new Error("Some Error")); const projectBuildContextMock = { possiblyRequiresBuild: possiblyRequiresBuildStub, - prepareProjectBuildAndValidateCache: prepareProjectBuildAndValidateCacheStub, + validateCache: validateCacheStub, buildProject: buildProjectStub, getProject: sinon.stub().returns(getMockProject("library")) }; @@ -293,7 +293,7 @@ test.serial("build: Multiple projects", async (t) => { const projectBuildContextMockA = { possiblyRequiresBuild: sinon.stub().returns(true), - prepareProjectBuildAndValidateCache: sinon.stub().resolves(false), + validateCache: sinon.stub().resolves(false), buildProject: buildProjectAStub, writeBuildCache: writeBuildCacheStub, getProject: sinon.stub().returns(getMockProject("library", "a")), @@ -301,7 +301,7 @@ test.serial("build: Multiple projects", async (t) => { }; const projectBuildContextMockB = { possiblyRequiresBuild: sinon.stub().returns(false), - prepareProjectBuildAndValidateCache: sinon.stub().resolves(false), + validateCache: sinon.stub().resolves(false), buildProject: buildProjectBStub, writeBuildCache: writeBuildCacheStub, getProject: sinon.stub().returns(getMockProject("library", "b")), @@ -309,7 +309,7 @@ test.serial("build: Multiple projects", async (t) => { }; const projectBuildContextMockC = { possiblyRequiresBuild: sinon.stub().returns(true), - prepareProjectBuildAndValidateCache: sinon.stub().resolves(false), + validateCache: sinon.stub().resolves(false), buildProject: buildProjectCStub, writeBuildCache: writeBuildCacheStub, getProject: sinon.stub().returns(getMockProject("library", "c")), diff --git a/packages/project/test/lib/build/cache/ProjectBuildCache.js b/packages/project/test/lib/build/cache/ProjectBuildCache.js index e09267db91b..82929a02ab7 100644 --- a/packages/project/test/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/test/lib/build/cache/ProjectBuildCache.js @@ -295,7 +295,7 @@ test("allTasksCompleted returns changed resource paths", async (t) => { const cache = await ProjectBuildCache.create(project, "sig", cacheManager); await cache.initSourceIndex(); - // Simulate some changes - change tracking happens during prepareProjectBuildAndValidateCache + // Simulate some changes - change tracking happens during validateCache cache.projectSourcesChanged(["/test.js"]); const changedPaths = await cache.allTasksCompleted(); @@ -802,14 +802,14 @@ test("projectSourcesChanged after SourceChangedDuringBuildError does not corrupt byGlobCallCount = 0; // Reset so initSourceIndex gets fresh resources await cache.initSourceIndex(); - // And prepareProjectBuildAndValidateCache should not crash + // And validateCache({prepareForBuild: true}) should not crash const mockDependencyReader = { byGlob: sinon.stub().resolves([]), byPath: sinon.stub().resolves(null) }; await t.notThrowsAsync( - () => cache.prepareProjectBuildAndValidateCache(mockDependencyReader), - "prepareProjectBuildAndValidateCache does not throw after race condition" + () => cache.validateCache(mockDependencyReader, {prepareForBuild: true}), + "validateCache does not throw after race condition" ); }); @@ -819,14 +819,14 @@ test("Retry after SourceChangedDuringBuildError when prior build set NO_CACHE: " // for project ...: no_cache". // // Sequence: - // 1. Initial build: prepareProjectBuildAndValidateCache validates the result cache, + // 1. Initial build: validateCache({prepareForBuild: true}) validates the result cache, // finds no match, transitions resultCacheState to NO_CACHE. // 2. allTasksCompleted detects a source change during build (file changed after the // build started but before the watcher's abort signal propagated). It throws // SourceChangedDuringBuildError and resets indexState — but historically did not // reset resultCacheState, leaving it stuck on NO_CACHE. - // 3. BuildServer re-enqueues the project. The retry's prepareProjectBuildAndValidateCache - // enters the RESTORING_DEPENDENCY_INDICES branch, which asserts resultCacheState === + // 3. BuildServer re-enqueues the project. The retry's validateCache enters the + // RESTORING_DEPENDENCY_INDICES branch, which asserts resultCacheState === // PENDING_VALIDATION. With the leftover NO_CACHE the assertion threw. const project = createMockProject(); const cacheManager = createMockCacheManager(); @@ -870,7 +870,7 @@ test("Retry after SourceChangedDuringBuildError when prior build set NO_CACHE: " }; // Step 1: initial build path — drives resultCacheState to NO_CACHE. - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); // Step 2: source changed during build — flip the source reader to the modified resource. revalidate = true; @@ -881,12 +881,12 @@ test("Retry after SourceChangedDuringBuildError when prior build set NO_CACHE: " revalidate = false; // pretend the file is stable on the retry await cache.initSourceIndex(); await t.notThrowsAsync( - () => cache.prepareProjectBuildAndValidateCache(mockDependencyReader), - "prepareProjectBuildAndValidateCache succeeds on retry" + () => cache.validateCache(mockDependencyReader, {prepareForBuild: true}), + "validateCache succeeds on retry" ); }); -test("prepareProjectBuildAndValidateCache: returns false for empty cache", async (t) => { +test("validateCache: prepareForBuild=true returns false for empty cache", async (t) => { const project = createMockProject(); const cacheManager = createMockCacheManager(); const cache = await ProjectBuildCache.create(project, "sig", cacheManager); @@ -897,7 +897,7 @@ test("prepareProjectBuildAndValidateCache: returns false for empty cache", async byPath: sinon.stub().resolves(null) }; - const result = await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + const result = await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); t.is(result, false, "Returns false for empty cache"); }); @@ -1546,10 +1546,10 @@ test("restoreFrozenSources: cache miss skips gracefully", async (t) => { byGlob: sinon.stub().resolves([]), byPath: sinon.stub().resolves(null) }; - const result = await cache.prepareProjectBuildAndValidateCache(mockDepReader); + const result = await cache.validateCache(mockDepReader, {prepareForBuild: true}); // Should succeed without error — cache miss for source stage is non-fatal - t.truthy(result, "prepareProjectBuildAndValidateCache succeeds despite source cache miss"); + t.truthy(result, "validateCache succeeds despite source cache miss"); // setFrozenSourceReader should NOT have been called t.false(project.getProjectResources().setFrozenSourceReader.called, @@ -1633,7 +1633,7 @@ test("restoreFrozenSources: cache hit creates CAS reader", async (t) => { byGlob: sinon.stub().resolves([]), byPath: sinon.stub().resolves(null) }; - const result = await cache.prepareProjectBuildAndValidateCache(mockDepReader); + const result = await cache.validateCache(mockDepReader, {prepareForBuild: true}); t.truthy(result, "Cache restored successfully"); @@ -1735,20 +1735,20 @@ async function createCacheInRestoringState({ return {cache, project, cacheManager, refreshSpy, mockDependencyReader}; } -test("prepareProjectBuildAndValidateCache: skips _refreshDependencyIndices when no dependency " + +test("validateCache prepareForBuild=true: skips _refreshDependencyIndices when no dependency " + "changes propagated (warm cache)", async (t) => { const {cache, refreshSpy, mockDependencyReader} = await createCacheInRestoringState(); // Do NOT call dependencyResourcesChanged — simulates warm cache with no upstream changes. // In RESTORING_DEPENDENCY_INDICES state, cached dependency indices (from BuildTaskCache.fromCache) // are already correct, so _refreshDependencyIndices can be skipped. - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); t.false(refreshSpy.called, "_refreshDependencyIndices should NOT be called when no dependency changes were propagated"); }); -test("prepareProjectBuildAndValidateCache: dependency changes move state from " + +test("validateCache prepareForBuild=true: dependency changes move state from " + "RESTORING_DEPENDENCY_INDICES to REQUIRES_UPDATE", async (t) => { const {cache, refreshSpy, mockDependencyReader} = await createCacheInRestoringState(); @@ -1758,7 +1758,7 @@ test("prepareProjectBuildAndValidateCache: dependency changes move state from " // the full _refreshDependencyIndices path. cache.dependencyResourcesChanged(["/dep/lib/SomeModule.js"]); - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); // _refreshDependencyIndices is NOT called because dependencyResourcesChanged() already moved // the state to REQUIRES_UPDATE, bypassing the RESTORING_DEPENDENCY_INDICES branch entirely. @@ -1767,28 +1767,28 @@ test("prepareProjectBuildAndValidateCache: dependency changes move state from " "_refreshDependencyIndices should NOT be called — changes handled via #flushPendingChanges"); }); -test("prepareProjectBuildAndValidateCache: transitions from RESTORING_DEPENDENCY_INDICES " + +test("validateCache prepareForBuild=true: transitions from RESTORING_DEPENDENCY_INDICES " + "to FRESH state", async (t) => { const {cache, refreshSpy, mockDependencyReader} = await createCacheInRestoringState(); // First call transitions from RESTORING_DEPENDENCY_INDICES to FRESH - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); t.false(refreshSpy.called, "_refreshDependencyIndices not called on first pass (no changes)"); // Second call without new changes — state is already FRESH, no refresh needed refreshSpy.resetHistory(); - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); t.false(refreshSpy.called, "_refreshDependencyIndices should NOT be called on second invocation (state is FRESH)"); }); -test("prepareProjectBuildAndValidateCache: subsequent dependency changes go through " + +test("validateCache prepareForBuild=true: subsequent dependency changes go through " + "REQUIRES_UPDATE path, not RESTORING_DEPENDENCY_INDICES", async (t) => { const {cache, refreshSpy, mockDependencyReader} = await createCacheInRestoringState(); // First call with no changes — transitions from RESTORING_DEPENDENCY_INDICES to FRESH - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); t.false(refreshSpy.called, "_refreshDependencyIndices not called on first pass (no changes)"); // Now simulate a dependency change (e.g. from BuildServer watch mode) @@ -1796,7 +1796,7 @@ test("prepareProjectBuildAndValidateCache: subsequent dependency changes go thro // Second call — should go through REQUIRES_UPDATE / #flushPendingChanges, not _refreshDependencyIndices refreshSpy.resetHistory(); - await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); t.false(refreshSpy.called, "_refreshDependencyIndices should NOT be called for REQUIRES_UPDATE state " + @@ -1847,21 +1847,6 @@ test("validateCache: warm cache without changes is fresh", async (t) => { t.is(result, false, "validateCache returns false when no result metadata is cached"); }); -test("validateCache: prepareProjectBuildAndValidateCache delegates to validateCache", async (t) => { - const {cache, mockDependencyReader} = await createCacheInRestoringState(); - - // Spy on validateCache to confirm prepareProjectBuildAndValidateCache delegates - const validateSpy = sinon.spy(cache, "validateCache"); - - const prepResult = await cache.prepareProjectBuildAndValidateCache(mockDependencyReader); - - t.true(validateSpy.calledOnce, "validateCache is invoked exactly once by prepareProjectBuildAndValidateCache"); - t.is(validateSpy.firstCall.args[0], mockDependencyReader, - "dependencyReader is forwarded to validateCache"); - t.is(prepResult, await validateSpy.firstCall.returnValue, - "prepareProjectBuildAndValidateCache returns whatever validateCache returned"); -}); - test("validateCache: Cache.Force throws when source changes are detected", async (t) => { // Build a warm cache pinned to Force whose initSourceIndex succeeds (no source changes // at init time), then introduce a change before invoking validateCache so the From ef7f1c264bea9a4eb1afa441670646cc29dfd533 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 6 Jul 2026 16:07:04 +0200 Subject: [PATCH 6/8] refactor(project): Consolidate BuildServer state-transition logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ad-hoc protocols around the VALIDATING state each move onto the type that owns them. Reader-request protocol → `ProjectBuildStatus`. It was spread across three call sites: `#getReaderForProject` skipped `#enqueueBuild` while `isValidating()`, `#runBackgroundValidation`'s finally re-enqueued a build when the project had pending reader requests, and `#triggerRequestQueue` called `#stopActiveValidation` to make room for the build. Each site carried a long block comment explaining why the others must bail. `ProjectBuildStatus` now takes an `onBuildRequired` callback at construction; BuildServer wires it to `#enqueueBuild(projectName)`. `releaseValidating()` fires the callback whenever the state isn't FRESH and the reader queue is non-empty, covering both the cache-stale case (VALIDATING → INITIAL with queued readers) and the invalidated-mid-pass case (INVALIDATED with queued readers). Call sites collapse: `#getReaderForProject` calls `addReaderRequest` unconditionally, `#runBackgroundValidation`'s finally shrinks to `status?.releaseValidating()`. `hasPendingReaderRequests()` had no other callers and goes away. Cycle-end reconciliation → `#reconcileServerState`. Both producer cycles (build cycle end, background validation pass end) open-coded the same terminal-state decision: bail if another actor already owns the next transition, then pick IDLE / VALIDATING / STALE based on the stale set. The post-validation finally carried a five-part guard whose intent ("only transition if nobody else has") was spread across three field checks and two state checks. `#reconcileServerState({hrtime, mayValidate} = {})` bails on `destroyed` or `ERROR`, then on `activeBuild || pendingBuildRequest.size > 0`. Otherwise: zero stale → IDLE; stale set non-empty → try `#scheduleBackgroundValidation` when `mayValidate` is true, fall back to STALE. Post-build passes `mayValidate: true`; post-validation passes `mayValidate: false` to avoid re-scheduling the pass that just settled. The old `state === BUILDING` branch is dropped. BUILDING is only set inside `#triggerRequestQueue`'s setTimeout callback, itself gated on `await #stopActiveValidation` — the same promise whose finally was performing the check. A future refactor introducing a synchronous BUILDING setter would need to re-add the guard. --- packages/project/lib/build/BuildServer.js | 176 +++++++++++++--------- 1 file changed, 105 insertions(+), 71 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 78dc1eebe95..d229a1dd576 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -107,10 +107,12 @@ class BuildServer extends EventEmitter { "Build Server: Dependencies Reader", dependencies, buildServerInterface); // Initialize cache states - this.#projectBuildStatus.set(this.#rootProjectName, new ProjectBuildStatus()); + this.#projectBuildStatus.set( + this.#rootProjectName, new ProjectBuildStatus(() => this.#enqueueBuild(this.#rootProjectName))); for (const dep of dependencies) { - this.#projectBuildStatus.set(dep.getName(), new ProjectBuildStatus()); + const depName = dep.getName(); + this.#projectBuildStatus.set(depName, new ProjectBuildStatus(() => this.#enqueueBuild(depName))); } } @@ -262,24 +264,20 @@ class BuildServer extends EventEmitter { return projectBuildStatus.getReader(); } const {promise, resolve, reject} = Promise.withResolvers(); + // Always queue the request on the status. It owns the "who resolves this" + // contract: a running validation pass drains its own queue via setReader + // on cache hit and via releaseValidating (which re-enqueues a build when + // the queue is non-empty) on cache miss. Callers uniformly enqueue and + // forget. projectBuildStatus.addReaderRequest({resolve, reject}); - // Skip the build-request enqueue while a background validation pass owns the - // project. If validation finds the cache fresh, it resolves the queued reader - // via setReader; if it finds the cache stale, #runBackgroundValidation enqueues - // a build for it (since pending reader requests can't wait for the next ad-hoc - // reader request). Enqueuing here unconditionally would fire #triggerRequestQueue's - // 10 ms timer, abort the validation pass mid-flight, and cause a needless - // VALIDATING → BUILDING → VALIDATING flicker even when validation would have - // served the request itself. - if (projectBuildStatus.isValidating()) { + if (!projectBuildStatus.isValidating()) { + log.verbose(`Reader for project '${projectName}' is not fresh. Enqueuing build request.`); + this.#enqueueBuild(projectName); + } else { log.verbose(`Reader for project '${projectName}' is validating. ` + `Waiting on validation pass.`); - return promise; } - - log.verbose(`Reader for project '${projectName}' is not fresh. Enqueuing build request.`); - this.#enqueueBuild(projectName); return promise; } @@ -430,27 +428,10 @@ class BuildServer extends EventEmitter { const hrtime = process.hrtime(cycleStart); // #processBuildRequests captures individual build failures on the // server state and continues the queue, so it resolves normally - // even when the build errored. Leave the state as-is (ERROR) and - // skip the post-build validation pass — the pass would surface - // yet more Force-mode / cache errors on top of the build error - // that already fired. - if (this.#serverState === SERVER_STATES.ERROR) { - return; - } - const staleProjects = this.#getStaleProjectNames(); - log.verbose(`Build cycle done. Stale projects: `+ - `${staleProjects.length} (${staleProjects.join(", ")})`); - if (staleProjects.length === 0) { - this.#setState(SERVER_STATES.IDLE, {hrtime}); - return; - } - // Some projects are still non-FRESH. Try to validate any that are merely - // INITIAL (cache validity unknown but possibly fresh) in the background. - // If a pass actually starts, the transition to VALIDATING happens inside - // #scheduleBackgroundValidation; otherwise we fall back to STALE. - if (!this.#scheduleBackgroundValidation({hrtime})) { - this.#setState(SERVER_STATES.STALE, {hrtime, staleProjects}); - } + // even when the build errored. #reconcileServerState's first check + // bails on state === ERROR, so the post-build validation pass is + // skipped without needing a second guard in #scheduleBackgroundValidation. + this.#reconcileServerState({hrtime, mayValidate: true}); }).catch((err) => { this.#setState(SERVER_STATES.ERROR, {error: err}); this.emit("error", err); @@ -587,6 +568,62 @@ class BuildServer extends EventEmitter { } } + /** + * Single point of truth for the terminal state at the end of a producer cycle + * (build cycle, background validation pass). Each producer that used to open-code + * an IDLE-vs-STALE-vs-VALIDATING guard now calls this after its own work settles; + * the reconciler picks the target state from the current fields — #activeBuild, + * #pendingBuildRequest, #serverState, the stale set — rather + * than trusting each producer to check them. + * + * Bails when another actor already owns the next transition: + * + * - #destroyed — server shutting down; state is irrelevant. + * - #serverState === ERROR — a producer already settled us on ERROR; + * don't paint over it with a successful cycle close. + * - #activeBuild || #pendingBuildRequest.size > 0 — a build cycle owns + * the next transition. Notably fires when this call comes from the post-validation + * finally and releaseValidating's onBuildRequired callback + * just enqueued a build for a project with pending readers. + * + * Otherwise: fully-fresh → IDLE. Any stale projects → try + * {@link #scheduleBackgroundValidation} when mayValidate is true and + * fall back to STALE. When called from the post-validation finally + * (mayValidate=false), goes straight to STALE — the pass just settled + * on those projects, so re-scheduling would be pointless and would recurse this + * finally into a new pass. + * + * @param {object} [opts] + * @param {Array} [opts.hrtime] Build duration to forward to + * {@link #setState}. Only the post-build path supplies this. + * @param {boolean} [opts.mayValidate=false] Whether the reconciler is allowed to + * schedule a background validation pass. Post-build → true, post-validation → false. + */ + #reconcileServerState({hrtime, mayValidate = false} = {}) { + if (this.#destroyed || this.#serverState === SERVER_STATES.ERROR) { + return; + } + if (this.#activeBuild || this.#pendingBuildRequest.size > 0) { + // Another producer (a build cycle) owns the next transition. + return; + } + const staleProjects = this.#getStaleProjectNames(); + log.verbose(`Reconciling server state. Stale projects: ` + + `${staleProjects.length} (${staleProjects.join(", ")})`); + if (staleProjects.length === 0) { + this.#setState(SERVER_STATES.IDLE, {hrtime}); + return; + } + // Some projects are still non-FRESH. Try to validate any that are merely + // INITIAL (cache validity unknown but possibly fresh) in the background. + // If a pass actually starts, the transition to VALIDATING happens inside + // #scheduleBackgroundValidation; otherwise we fall back to STALE. + if (mayValidate && this.#scheduleBackgroundValidation({hrtime})) { + return; + } + this.#setState(SERVER_STATES.STALE, {hrtime, staleProjects}); + } + /** * Kick off a background cache-validation pass for projects that have never been built or * validated yet. Runs as a fire-and-forget promise; the result is tracked on @@ -644,22 +681,9 @@ class BuildServer extends EventEmitter { .finally(() => { this.#activeValidation = null; this.#validationAbort = null; - // If a build is queued or running, leave state to #triggerRequestQueue's - // BUILDING transition. Otherwise (and unless validation failed — in which - // case the catch above has already settled state on ERROR) reflect the - // outcome of the validation pass: fully fresh → IDLE, anything else → STALE. - if (this.#destroyed || this.#activeBuild || - this.#pendingBuildRequest.size > 0 || - this.#serverState === SERVER_STATES.BUILDING || - this.#serverState === SERVER_STATES.ERROR) { - return; - } - const staleProjects = this.#getStaleProjectNames(); - if (staleProjects.length === 0) { - this.#setState(SERVER_STATES.IDLE); - } else { - this.#setState(SERVER_STATES.STALE, {staleProjects}); - } + // mayValidate=false: a validation pass just finished; it would be pointless + // (and stack-recursive) to schedule another for the projects it left non-FRESH. + this.#reconcileServerState({mayValidate: false}); }); return true; } @@ -704,19 +728,13 @@ class BuildServer extends EventEmitter { // // Reader requests issued during VALIDATING skip #enqueueBuild (see // #getReaderForProject) so the validation pass can run to completion without being - // aborted by its own queue tick. Pick up the slack here: any project the pass - // didn't promote to FRESH but which has callers waiting on a reader must now be - // built. #enqueueBuild is idempotent, so re-enqueueing one already scheduled by - // the watcher or a concurrent caller is harmless. + // aborted by its own queue tick. releaseValidating owns the pick-up-the-slack side + // of that contract: if the project didn't reach FRESH and callers are still waiting + // on a reader, it invokes the onBuildRequired callback wired at construction, which + // enqueues a build. #enqueueBuild is idempotent, so re-enqueueing one already + // scheduled by the watcher or a concurrent caller is harmless. for (const projectName of projectNames) { - const status = this.#projectBuildStatus.get(projectName); - if (!status) { - continue; - } - status.releaseValidating(); - if (!status.isFresh() && status.hasPendingReaderRequests()) { - this.#enqueueBuild(projectName); - } + this.#projectBuildStatus.get(projectName)?.releaseValidating(); } } } @@ -793,6 +811,18 @@ class ProjectBuildStatus { #readerQueue = []; #reader; #abortController = new AbortController(); + #onBuildRequired; + + /** + * @param {Function} [onBuildRequired] Invoked when the status leaves the VALIDATING + * phase without becoming FRESH while at least one reader request is queued. + * The owning {@link BuildServer} wires this to #enqueueBuild so the + * status alone decides when a follow-up build is required — reader-request + * handling stays a single-owner protocol instead of a three-site coordination. + */ + constructor(onBuildRequired) { + this.#onBuildRequired = onBuildRequired; + } /** * Flip the project to INVALIDATED, aborting any in-flight build. @@ -865,14 +895,22 @@ class ProjectBuildStatus { /** * Reverts a VALIDATING project back to INITIAL — used when validation found the - * cache to be stale and the project should remain lazy. No-op for any other state - * (e.g. when invalidate() already moved the project to INVALIDATED mid-validation, - * or a build claimed it in the meantime). + * cache to be stale and the project should remain lazy. No-op for the state + * transition on any other state (e.g. when invalidate() already moved the + * project to INVALIDATED mid-validation, or a build claimed it in the meantime). + * + * Regardless of the prior state, if the project is not FRESH and reader requests + * are still queued, fires the onBuildRequired callback: those callers + * skipped enqueueing a build themselves while the pass owned the project, so + * releasing without resolving them requires a follow-up build. */ releaseValidating() { if (this.#state === PROJECT_STATES.VALIDATING) { this.#state = PROJECT_STATES.INITIAL; } + if (this.#state !== PROJECT_STATES.FRESH && this.#readerQueue.length > 0) { + this.#onBuildRequired?.(); + } } abortBuild(reason) { @@ -918,10 +956,6 @@ class ProjectBuildStatus { this.#readerQueue.push(promiseResolvers); } - hasPendingReaderRequests() { - return this.#readerQueue.length > 0; - } - rejectReaderRequests(error) { this.#state = PROJECT_STATES.INVALIDATED; for (const {reject} of this.#readerQueue) { From 5e57496553bd932fbde6d414e944fdd179b18ba7 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 6 Jul 2026 12:52:58 +0200 Subject: [PATCH 7/8] refactor(project): Guard #triggerRequestQueue against concurrent build claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the request-queue timer awaits `#stopActiveValidation`, validation's finally can call `releaseValidating` → `onBuildRequired` → schedule a second timer. When that timer fires with `#activeBuild` already set, it calls `projectBuilder.build` concurrently; the builder's `buildIsRunning` guard rejects with 'A build is already running' and the server transitions to ERROR, emitting a spurious 'error' event. Re-check `#activeBuild` after the await. The active build's `#reconcileServerState` hook drains `#pendingBuildRequest` on its own. --- packages/project/lib/build/BuildServer.js | 7 + .../project/test/lib/build/BuildServer.js | 205 ++++++++++++++++++ 2 files changed, 212 insertions(+) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index d229a1dd576..5b157f0e013 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -422,6 +422,13 @@ class BuildServer extends EventEmitter { if (this.#destroyed) { return; } + // A concurrent timer may have claimed the build slot during the await + // above — validation's finally can fire onBuildRequired, which schedules + // a second timer. Bail so we don't call projectBuilder.build twice; the + // active build's #reconcileServerState hook drains #pendingBuildRequest. + if (this.#activeBuild) { + return; + } const cycleStart = process.hrtime(); this.#setState(SERVER_STATES.BUILDING); this.#processBuildRequests().then(() => { diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 644ce31c6a0..9e9200a978d 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -744,4 +744,209 @@ test.serial( await clock.tickAsync(20); }); +// Regression test for a race where the request-queue timer schedules a second +// timer (T2) during T1's `await #stopActiveValidation`. Without a re-check of +// `#activeBuild` after that await, T2 would call `#processBuildRequests` while +// T1's build is still in flight, invoke `projectBuilder.build` concurrently, and +// surface a spurious "A build is already running" error to the user. +// +// Sequence exercised below: +// 1. Initial build of root completes; library.x and library.y enter +// background VALIDATING. +// 2. Reader request lands on library.x — queued on its readerQueue via the +// isValidating() branch, so no build is enqueued. +// 3. Source change on root invalidates root only (traverseDependents yields +// just root); library.x/y stay in VALIDATING. +// 4. Reader request for root enqueues a build → schedules T1. +// 5. T1 fires: `#stopActiveValidation` aborts the pass. The pass's finally +// releases library.x/y, which fires `onBuildRequired` for library.x +// (queued reader) — that re-enters `#triggerRequestQueue` and schedules T2 +// while `#activeBuild` is still null. +// 6. T1 resumes, claims `#activeBuild`, calls `projectBuilder.build`. +// 7. A reader request for library.y arrives during T1's build → +// `#pendingBuildRequest = {library.y}`. +// 8. T2 fires 10 ms later. Without the guard, it calls +// `projectBuilder.build` a second time while T1's is still pending. +test.serial( + "triggerRequestQueue: does not fire a second build while T1's build is in flight", + async (t) => { + const {sinon, clock} = t.context; + + const rootProject = {getName: () => "root.project"}; + const libX = {getName: () => "library.x"}; + const libY = {getName: () => "library.y"}; + const projectsByName = { + "root.project": rootProject, + "library.x": libX, + "library.y": libY, + }; + // traverseDependents yields only the source project itself so a source + // change on root does not cascade into library.x or library.y. + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libX, libY], + getTransitiveDependencies: () => ["library.x", "library.y"], + getProject: (name) => projectsByName[name], + traverseDependents: function* (projectName) { + yield {project: projectsByName[projectName]}; + }, + }; + + // Track concurrent invocations of projectBuilder.build. The second + // concurrent call rejects with the same error the real ProjectBuilder + // would throw synchronously from its "buildIsRunning" guard. + let inFlightBuilds = 0; + let maxConcurrentBuilds = 0; + const buildInvocations = []; + + // Validation pass hangs until aborted; the abort signal's rejection is what + // #stopActiveValidation awaits. + const validationGate = (signal) => new Promise((_, reject) => { + if (signal.aborted) { + reject(signal.reason); + return; + } + signal.addEventListener("abort", () => reject(signal.reason), {once: true}); + }); + + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().callsFake(async ({willValidate, signal}) => { + willValidate?.("library.x"); + willValidate?.("library.y"); + // Wait until the pass is aborted by #stopActiveValidation. The + // re-thrown abort reason is caught by #runBackgroundValidation. + await validationGate(signal); + }), + build: sinon.stub().callsFake((opts, perProjectCb) => { + if (inFlightBuilds > 0) { + return Promise.reject(new Error("A build is already running")); + } + inFlightBuilds++; + maxConcurrentBuilds = Math.max(maxConcurrentBuilds, inFlightBuilds); + const {promise, resolve} = Promise.withResolvers(); + buildInvocations.push({opts, perProjectCb, resolve}); + return promise.finally(() => { + inFlightBuilds--; + }); + }), + }; + + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + + const BuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + const statusEvents = makeStatusRecorder(t); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); + // Swallow the emitted error so AVA doesn't see an unhandled rejection + // if the race actually fires (which it will, without the fix). + const errorEvents = []; + buildServer.on("error", (err) => errorEvents.push(err)); + + // Drive the initial build of root. + await clock.tickAsync(10); + // Resolve the initial root build. + t.is(buildInvocations.length, 1, "initial root build was invoked"); + buildInvocations[0].perProjectCb("root.project", { + getReader: () => ({builtReader: "root.project"}), + }); + buildInvocations[0].resolve(["root.project"]); + + // Wait for the post-build VALIDATING transition. + await drainUntil(clock, statusEvents, (e) => e.status === "serve-validating"); + + // (2) Reader request on VALIDATING library.x — queued, no enqueueBuild. + const libXReaderPromise = capturedInterface.getReaderForProject("library.x"); + // Keep the promise unhandled from AVA's perspective by attaching a noop + // catch. If the fix is in place this resolves cleanly at teardown; if + // not, it may reject when a follow-up build fails. + libXReaderPromise.catch(() => {}); + + // (3) Source change on root: only invalidates root (traverseDependents + // yields root alone), leaving library.x/y in VALIDATING. + buildServer._projectResourceChanged(rootProject, "/foo.js", false); + + // (4) Reader request for root → root is INVALIDATED, so this enqueues a + // build and schedules T1. + const rootReaderPromise = capturedInterface.getReaderForProject("root.project"); + rootReaderPromise.catch(() => {}); + + // (5) Fire T1. tickAsync(10) drains through the timer body's await, which + // aborts validation; validation's finally releases library.x's queued + // reader via onBuildRequired → enqueueBuild(library.x) → schedules T2. + // T1 then continues, claims #activeBuild, and calls projectBuilder.build + // (the second recorded invocation). + await clock.tickAsync(10); + + t.is(buildInvocations.length, 2, "T1 kicked off a second projectBuilder.build call"); + const t1Build = buildInvocations[1]; + t.true(t1Build.opts.includeRootProject, "T1 builds root"); + t.deepEqual(t1Build.opts.includedDependencies, ["library.x"], + "T1 also picks up library.x from the onBuildRequired re-entry"); + + // (7) Reader request for library.y while T1's build is pending. This + // populates #pendingBuildRequest but triggerRequestQueue early-returns + // because #activeBuild != null. + const libYReaderPromise = capturedInterface.getReaderForProject("library.y"); + libYReaderPromise.catch(() => {}); + + // (8) Fire T2. Without the fix, T2's timer body would call + // projectBuilder.build concurrently and the mock would reject with + // "A build is already running" (mirroring the real ProjectBuilder). + await clock.tickAsync(10); + + // Primary assertion: at no point were two builds in flight. + t.is(maxConcurrentBuilds, 1, + "projectBuilder.build must never be called while a build is already in flight"); + + // Observable surface: no spurious serve-error / error event. + const seq = statusEvents.map((e) => e.status); + t.false(seq.includes("serve-error"), + `No serve-error should be emitted; got: ${seq.join(", ")}`); + t.is(errorEvents.length, 0, "No BuildServer 'error' event should be emitted"); + + // Let T1's build finish. The follow-up library.y build then runs in the + // same processBuildRequests cycle. Resolve each in order and drain. + t1Build.perProjectCb("root.project", {getReader: () => ({builtReader: "root.project"})}); + t1Build.perProjectCb("library.x", {getReader: () => ({builtReader: "library.x"})}); + t1Build.resolve(["root.project", "library.x"]); + + // Drain until the library.y build lands. + for (let i = 0; i < 100 && buildInvocations.length < 3; i++) { + await clock.tickAsync(1); + } + t.is(buildInvocations.length, 3, "library.y is built in the same request-queue cycle"); + const libYBuild = buildInvocations[2]; + libYBuild.perProjectCb("library.y", {getReader: () => ({builtReader: "library.y"})}); + libYBuild.resolve(["library.y"]); + + await drainUntil(clock, statusEvents, (e) => e.status === "serve-ready"); + + // All three reader requests must have resolved. + t.deepEqual(await libXReaderPromise, {builtReader: "library.x"}, + "library.x reader resolved by its follow-up build"); + t.deepEqual(await rootReaderPromise, {builtReader: "root.project"}, + "root reader resolved by T1's build"); + t.deepEqual(await libYReaderPromise, {builtReader: "library.y"}, + "library.y reader resolved by the trailing build"); + }); + From 6f2fae14997dcf72555b66cacca787375aa7b064 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Wed, 8 Jul 2026 16:10:39 +0200 Subject: [PATCH 8/8] refactor(project): Consolidate abort checks to ProjectBuilder --- packages/project/lib/build/BuildServer.js | 3 --- packages/project/lib/build/ProjectBuilder.js | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 5b157f0e013..22cc4bd873a 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -709,9 +709,6 @@ class BuildServer extends EventEmitter { projectBuildStatus?.markValidating(); }, }, (projectName, project, projectBuildContext, usesCache) => { - if (signal.aborted) { - return; - } const projectBuildStatus = this.#projectBuildStatus.get(projectName); if (!projectBuildStatus) { return; diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index 4d5792d87d9..94a724b34fd 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -415,6 +415,7 @@ class ProjectBuilder { const startTime = process.hrtime(); try { for (const projectBuildContext of queue) { + signal?.throwIfAborted(); const project = projectBuildContext.getProject(); const projectName = project.getName(); const projectType = project.getType(); @@ -432,6 +433,7 @@ class ProjectBuilder { `completed in ${(performance.now() - prepStart).toFixed(2)} ms ` + `(usesCache=${usesCache})`); } + signal?.throwIfAborted(); if (usesCache) { this.#log.skipProjectBuild(projectName, projectType); alreadyBuilt.push(projectName); @@ -550,6 +552,8 @@ class ProjectBuilder { } } + signal?.throwIfAborted(); + if (projectValidatedCallback && isRequested) { await projectValidatedCallback(projectName, project, projectBuildContext, usesCache); }