From d40f3fb658838914b4e8221a3990fec005360780 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:40:25 +0800 Subject: [PATCH] test(hooks): synchronize inspection of an unpublished install lock The Windows coverage job can schedule the test after the old 200 ms publication delay has elapsed. File existence then no longer proves an empty lock, so the setup assertion fails even though the installer is correct. The original implementation reproduces that failure when parent inspection is held until the record has been published. Replace the delay-only test hook with two opt-in fixture barriers. Hold the publisher after exclusive creation, and acknowledge the contender only after it has observed an incomplete record. The test checks the empty bytes before releasing both processes and always releases and awaits them during cleanup. Existing normal-path lock and initialization budgets are unchanged; no test barrier is entered without its explicit environment setting. The focused barrier case and all 38 installer tests pass, as do scoped lint and whitespace checks. This changes test synchronization rather than loosening lock ownership or increasing production timeouts. --- scripts/install-lefthook.mjs | 22 +++++++++++++++++---- scripts/install-lefthook.spec.ts | 33 +++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/scripts/install-lefthook.mjs b/scripts/install-lefthook.mjs index 3f8a4904ed..172bd89a39 100644 --- a/scripts/install-lefthook.mjs +++ b/scripts/install-lefthook.mjs @@ -378,21 +378,30 @@ function releaseInstallLock(lockPath, ownedRecord, ownedStat) { } } +/** Hold one opt-in fixture stage until its parent explicitly releases it. */ +async function waitForLockTestBarrier(path) { + writeFileSync(`${path}.ready`, '') + const deadline = Date.now() + INSTALL_LOCK_TIMEOUT_MS + while (!existsSync(`${path}.release`)) { + if (Date.now() >= deadline) throw new Error(`timed out waiting for installer test barrier ${path}`) + await new Promise(resolveWait => setTimeout(resolveWait, INSTALL_LOCK_POLL_MS)) + } +} + async function acquireInstallLock(commonDirectory) { const lockPath = join(commonDirectory, INSTALL_LOCK) const deadline = Date.now() + INSTALL_LOCK_TIMEOUT_MS const ownedRecord = `${String(process.pid)} ${randomUUID()}\n` let initializingLock + let observeBarrier = process.env.DSH_TEST_LEFTHOOK_LOCK_OBSERVE_BARRIER while (true) { try { const lockHandle = openSync(lockPath, 'wx', 0o600) let ownedStat try { ownedStat = fstatSync(lockHandle) - const writeDelay = Number(process.env.DSH_TEST_LEFTHOOK_LOCK_WRITE_DELAY_MS ?? 0) - if (writeDelay > 0) { - await new Promise(resolveWait => setTimeout(resolveWait, writeDelay)) - } + const publicationBarrier = process.env.DSH_TEST_LEFTHOOK_LOCK_PUBLISH_BARRIER + if (publicationBarrier !== undefined) await waitForLockTestBarrier(publicationBarrier) writeFileSync(lockHandle, ownedRecord) } finally { closeSync(lockHandle) @@ -428,6 +437,11 @@ async function acquireInstallLock(commonDirectory) { if (!installLockRecordMayBeIncomplete(existingRecord)) { throw manualLockRecoveryError(lockPath, 'invalid') } + if (observeBarrier !== undefined) { + const barrier = observeBarrier + observeBarrier = undefined + await waitForLockTestBarrier(barrier) + } const now = Date.now() if ( initializingLock === undefined diff --git a/scripts/install-lefthook.spec.ts b/scripts/install-lefthook.spec.ts index b6b2831fb6..a05cb8cf2a 100644 --- a/scripts/install-lefthook.spec.ts +++ b/scripts/install-lefthook.spec.ts @@ -348,17 +348,32 @@ describe('worktree-local Lefthook installer', { timeout: 90_000 }, () => { it('waits for a concurrent installer to finish publishing its lock record', async () => { const fixture = createFixture() const lockPath = installLockPath(fixture) + const publicationBarrier = join(fixture.container, 'lock-publication') + const observationBarrier = join(fixture.container, 'lock-observation') const publishing = runInstaller(fixture, fixture.main, { - DSH_TEST_LEFTHOOK_LOCK_WRITE_DELAY_MS: '200', + DSH_TEST_LEFTHOOK_LOCK_PUBLISH_BARRIER: publicationBarrier, }) - await waitForPath(lockPath) - expect(readFileSync(lockPath, 'utf8')).toBe('') - - const waiting = runInstaller(fixture, fixture.linked) - const results = await Promise.all([publishing, waiting]) - - for (const result of results) expect(result.status, result.stderr).toBe(0) - expect(existsSync(lockPath)).toBe(false) + let waiting: Promise | undefined + const release = (): void => { + writeFileSync(`${publicationBarrier}.release`, '') + writeFileSync(`${observationBarrier}.release`, '') + } + try { + await waitForPath(`${publicationBarrier}.ready`) + expect(readFileSync(lockPath, 'utf8')).toBe('') + waiting = runInstaller(fixture, fixture.linked, { + DSH_TEST_LEFTHOOK_LOCK_OBSERVE_BARRIER: observationBarrier, + }) + await waitForPath(`${observationBarrier}.ready`) + expect(readFileSync(lockPath, 'utf8')).toBe('') + release() + const results = await Promise.all([publishing, waiting]) + for (const result of results) expect(result.status, result.stderr).toBe(0) + expect(existsSync(lockPath)).toBe(false) + } finally { + release() + await Promise.allSettled([publishing, ...waiting === undefined ? [] : [waiting]]) + } }) it('repairs its owned absolute hook path after the checkout moves', async () => {