Skip to content

Commit 6cc60af

Browse files
committed
test(project): De-flake "Serve library" and document StageCache abort bug
The "Serve library" test could flake when an FS event from the fixture copy fired after chokidar's ready signal, aborting the initial build and triggering a retry. The retry hit in-memory StageCache entries left behind by the aborted attempt and emitted spurious task-skip events, breaking the first-request assertion. Decouple fixture initialization from serveProject. FixtureTester now exposes a static create() factory that runs the rmrf + fs.cp up front, and serveProject only constructs the graph and starts the build server. The graph computation between cp and watcher attach gives the OS event queue time to flush, so the watcher does not see late events from the copy. Add two test.serial.failing tests that deterministically reproduce the StageCache-on-abort bug by triggering an abort at a known task boundary via _projectResourceChanged. The variants cover early (after task 4) and late (after task 7) abort points to show the leak scales with how far the aborted attempt progressed.
1 parent f8507c3 commit 6cc60af

1 file changed

Lines changed: 117 additions & 17 deletions

File tree

packages/project/test/lib/build/BuildServer.integration.js

Lines changed: 117 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ test.afterEach.always(async (t) => {
4242
// once the BuildServer has been started and built a project at least once.
4343
// This is independent of caching on file-system level, which is isolated per test via tmp folders.
4444
test.serial("Serve application.a, initial file changes", async (t) => {
45-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
45+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
4646

4747
await fixtureTester.serveProject();
4848

@@ -77,7 +77,7 @@ test.serial("Serve application.a, initial file changes", async (t) => {
7777
});
7878

7979
test.serial("Serve application.a, request application resource", async (t) => {
80-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
80+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
8181

8282
// #1 request with empty cache
8383
await fixtureTester.serveProject();
@@ -128,7 +128,7 @@ test.serial("Serve application.a, request application resource", async (t) => {
128128
});
129129

130130
test.serial("Serve application.a, request library resource", async (t) => {
131-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
131+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
132132

133133
// #1 request with empty cache
134134
await fixtureTester.serveProject();
@@ -201,7 +201,7 @@ test.serial("Serve application.a, request library resource", async (t) => {
201201
});
202202

203203
test.serial("Serve library", async (t) => {
204-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "library.d");
204+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "library.d");
205205

206206
// #1 request with empty cache
207207
await fixtureTester.serveProject({
@@ -285,7 +285,7 @@ test.serial("Serve library", async (t) => {
285285
});
286286

287287
test.serial("Serve application.a, request application resource AND library resource", async (t) => {
288-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
288+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
289289

290290
// #1 request with empty cache
291291
await fixtureTester.serveProject();
@@ -357,7 +357,7 @@ test.serial("Serve application.a, request application resource AND library resou
357357
});
358358

359359
test.serial("Serve application.a with --cache=Default", async (t) => {
360-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
360+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
361361

362362
// #1: Serve and request with empty cache --> all tasks execute
363363
await fixtureTester.serveProject({config: {cache: Cache.Default}});
@@ -409,7 +409,7 @@ test.serial("Serve application.a with --cache=Default", async (t) => {
409409
});
410410

411411
test.serial("Serve application.a with --cache=Off", async (t) => {
412-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
412+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
413413

414414
// #1: Serve and request with cache=Off --> all tasks execute, cache not written
415415
await fixtureTester.serveProject({config: {cache: Cache.Off}});
@@ -489,7 +489,7 @@ test.serial("Serve application.a with --cache=Off", async (t) => {
489489
});
490490

491491
test.serial("Serve application.a with --cache=ReadOnly", async (t) => {
492-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
492+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
493493

494494
// #1: Serve and request with cache=Default --> all tasks execute, cache written
495495
await fixtureTester.serveProject({config: {cache: Cache.Default}});
@@ -567,7 +567,7 @@ test.serial("Serve application.a with --cache=ReadOnly", async (t) => {
567567
});
568568

569569
test.serial("Serve application.a with --cache=Force (1)", async (t) => {
570-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
570+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
571571

572572
// #1: Serve and request with cache=Default --> all tasks execute, cache written
573573
await fixtureTester.serveProject({config: {cache: Cache.Default}});
@@ -613,7 +613,7 @@ test.serial("Serve application.a with --cache=Force (1)", async (t) => {
613613
});
614614

615615
test.serial("Serve application.a with --cache=Force (2)", async (t) => {
616-
const fixtureTester = t.context.fixtureTester = new FixtureTester(t, "application.a");
616+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a");
617617

618618
// #1: Serve with cache=Force on empty cache --> ERROR when requesting resource
619619
await fixtureTester.serveProject({config: {cache: Cache.Force}, expectBuildErrors: true});
@@ -631,6 +631,104 @@ test.serial("Serve application.a with --cache=Force (2)", async (t) => {
631631
await setTimeout(50);
632632
});
633633

634+
// ProjectBuildCache's StageCache must be cleared correctly when a build is aborted.
635+
// A task that completed during an aborted attempt has already called recordTaskResult,
636+
// which adds its stage to the in-memory StageCache. On retry,
637+
// prepareTaskExecutionAndValidateCache might finds those entries via #findStageCache if not cleaned up.
638+
// It will then emit task-skip events for tasks that the retry should have actually re-executed.
639+
test.serial.failing("Aborted initial build must not leak in-memory StageCache to retry", async (t) => {
640+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "library.d");
641+
await fixtureTester.serveProject({
642+
config: {excludedTasks: ["minify"]}
643+
});
644+
const project = fixtureTester.graph.getProject("library.d");
645+
646+
// One-shot trigger: when `replaceBuildtime` (the 4th task for this fixture) ends in the
647+
// initial build, simulate a watcher event by calling _projectResourceChanged directly.
648+
// This invalidates library.d, aborts the running build at the next signal check, and
649+
// re-enqueues it. By that point, tasks 1-4 have completed recordTaskResult and live in
650+
// the in-memory StageCache. Tasks 5+ never started.
651+
let aborted = false;
652+
const abortHandler = (event) => {
653+
if (
654+
!aborted &&
655+
event.projectName === "library.d" &&
656+
event.status === "task-end" &&
657+
event.taskName === "replaceBuildtime"
658+
) {
659+
aborted = true;
660+
fixtureTester.buildServer._projectResourceChanged(
661+
project, "/resources/library/d/some.js", false);
662+
}
663+
};
664+
process.on("ui5.project-build-status", abortHandler);
665+
666+
try {
667+
// byPath returns once the retry succeeds, so all events for both attempts are captured.
668+
await fixtureTester._reader.byPath("/resources/library/d/some.js");
669+
} finally {
670+
process.off("ui5.project-build-status", abortHandler);
671+
}
672+
673+
t.true(aborted, "Test setup precondition: abort trigger should have fired");
674+
675+
// On a fresh fixture the persistent cache is empty. After the fix, the retry's
676+
// prepareTaskExecutionAndValidateCache should find no cached stages (in-memory cache
677+
// from the aborted build is discarded) and execute every task. No task-skip events
678+
// should be emitted for library.d.
679+
const skippedTasks = t.context.projectBuildStatusEventStub.args
680+
.map(([event]) => event)
681+
.filter((e) => e.projectName === "library.d" && e.status === "task-skip")
682+
.map((e) => e.taskName);
683+
684+
t.deepEqual(skippedTasks, [],
685+
"Persistent cache is empty and the in-memory StageCache populated by the aborted " +
686+
"attempt must not be reused on retry");
687+
});
688+
689+
// Same scenario as above but the for a later abort: After `generateLibraryPreload`
690+
test.serial.failing(
691+
"Aborted initial build leaks all StageCache entries up to the abort point", async (t) => {
692+
const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "library.d");
693+
await fixtureTester.serveProject({
694+
config: {excludedTasks: ["minify"]}
695+
});
696+
const project = fixtureTester.graph.getProject("library.d");
697+
698+
let aborted = false;
699+
const abortHandler = (event) => {
700+
if (
701+
!aborted &&
702+
event.projectName === "library.d" &&
703+
event.status === "task-end" &&
704+
event.taskName === "generateLibraryPreload"
705+
) {
706+
aborted = true;
707+
fixtureTester.buildServer._projectResourceChanged(
708+
project, "/resources/library/d/some.js", false);
709+
}
710+
};
711+
process.on("ui5.project-build-status", abortHandler);
712+
713+
try {
714+
await fixtureTester._reader.byPath("/resources/library/d/some.js");
715+
} finally {
716+
process.off("ui5.project-build-status", abortHandler);
717+
}
718+
719+
t.true(aborted, "Test setup precondition: abort trigger should have fired");
720+
721+
const skippedTasks = t.context.projectBuildStatusEventStub.args
722+
.map(([event]) => event)
723+
.filter((e) => e.projectName === "library.d" && e.status === "task-skip")
724+
.map((e) => e.taskName);
725+
726+
t.deepEqual(skippedTasks, [],
727+
"Persistent cache is empty and the in-memory StageCache populated by the aborted " +
728+
"attempt must not be reused on retry");
729+
}
730+
);
731+
634732
function getFixturePath(fixtureName) {
635733
return fileURLToPath(new URL(`../../fixtures/${fixtureName}`, import.meta.url));
636734
}
@@ -644,11 +742,19 @@ async function rmrf(dirPath) {
644742
}
645743

646744
class FixtureTester {
745+
// Initialization (rmrf + fs.cp of the fixture into the tmp directory) is done up-front
746+
// and separately from `serveProject`, so that the build server's file watcher does not
747+
// race with FS events from the copy.
748+
static async create(t, fixtureName) {
749+
const fixtureTester = new FixtureTester(t, fixtureName);
750+
await fixtureTester._initialize();
751+
return fixtureTester;
752+
}
753+
647754
constructor(t, fixtureName) {
648755
this._t = t;
649756
this._sinon = t.context.sinon;
650757
this._fixtureName = fixtureName;
651-
this._initialized = false;
652758

653759
// Public
654760
this.fixturePath = getTmpPath(fixtureName);
@@ -657,13 +763,9 @@ class FixtureTester {
657763
}
658764

659765
async _initialize() {
660-
if (this._initialized) {
661-
return;
662-
}
663766
process.env.UI5_DATA_DIR = getTmpPath(`${this._fixtureName}/.ui5`);
664767
await rmrf(this.fixturePath); // Clean up any previous test runs
665768
await fs.cp(getFixturePath(this._fixtureName), this.fixturePath, {recursive: true});
666-
this._initialized = true;
667769
}
668770

669771
async teardown() {
@@ -677,8 +779,6 @@ class FixtureTester {
677779
}
678780

679781
async serveProject({graphConfig = {}, config = {}, expectBuildErrors = false} = {}) {
680-
await this._initialize();
681-
682782
const graph = this.graph = await graphFromPackageDependencies({
683783
...graphConfig,
684784
cwd: this.fixturePath,

0 commit comments

Comments
 (0)