Skip to content

Commit 6260652

Browse files
committed
fix(assets): limit the live watcher to one test and defer temp-dir cleanup
The first attempt (a 50ms post-close grace period) wasn't enough on Windows CI runners — every one of the 13 tests in assets.test.ts was opening, closing, and immediately deleting a live-watched temp dir, each one a fresh chance at the libuv fs-event race. - Only the test that actually exercises the watcher now passes watch: true; the other 12 opt out (watch: false), so this file opens exactly one native Windows directory watch per run. - Every temp dir's deletion is deferred to a single afterAll instead of each test's afterEach, so cleanup runs well clear of that one watcher's teardown. - Bumped the win32 post-close grace period from 50ms to 200ms.
1 parent 3876d41 commit 6260652

2 files changed

Lines changed: 35 additions & 19 deletions

File tree

plugins/assets/src/node/watcher.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ export function watchAssetsDir(ctx: DevframeNodeContext, dir: string): () => Pro
3434
await watcher.close()
3535
// On Windows, closing a chokidar/fs.watch handle doesn't guarantee the
3636
// OS has fully retired the outstanding ReadDirectoryChangesW request —
37-
// ftruncate/close returns before libuv's IOCP completion drains. If the
38-
// watched directory is deleted immediately after (as every test's
39-
// `afterEach` does), that stale completion can reach
37+
// close() returns before libuv's IOCP completion drains. If the watched
38+
// directory is deleted right after (as a caller tearing down a
39+
// short-lived context typically does), that stale completion can reach
4040
// `uv__fs_event_process` after the fact and trip its directory-prefix
4141
// sanity check, hard-crashing the process with
4242
// `Assertion failed: !_wcsnicmp(filename, dir, dirlen)` in fs-event.c.
43-
// A short grace period gives the pending I/O time to actually settle.
43+
// A grace period gives the pending I/O time to actually settle first.
4444
if (process.platform === 'win32')
45-
await new Promise(resolve => setTimeout(resolve, 50))
45+
await new Promise(resolve => setTimeout(resolve, 200))
4646
}
4747
}

plugins/assets/test/assets.test.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { AssetsServer, TestClient } from './_utils'
22
import { Buffer } from 'node:buffer'
33
import fsp from 'node:fs/promises'
44
import { join } from 'node:path'
5-
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
5+
import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'
66
import { bootClient, call, cleanupTempDir, createTempDir, startAssetsServer } from './_utils'
77

88
// A minimal, valid 1x1 PNG.
@@ -36,31 +36,44 @@ describe('assets plugin', () => {
3636
let server: AssetsServer
3737
let client: TestClient
3838

39+
// Every managed temp dir this file creates, deleted once at the very end
40+
// rather than per-test. On Windows, closing a live chokidar watcher
41+
// doesn't guarantee libuv has retired the outstanding
42+
// ReadDirectoryChangesW request — deleting the directory it watched right
43+
// after close() can trip a native assertion and hard-crash the process
44+
// (see `watchAssetsDir`'s disposer). Deferring every directory's removal
45+
// well past its server's shutdown keeps that race out of this suite.
46+
const tempDirs: string[] = []
47+
3948
beforeEach(async () => {
4049
dir = await createTempDir()
50+
tempDirs.push(dir)
4151
})
4252

4353
afterEach(async () => {
4454
await server?.close()
45-
await cleanupTempDir(dir)
55+
})
56+
57+
afterAll(async () => {
58+
await Promise.all(tempDirs.map(cleanupTempDir))
4659
})
4760

4861
it('lists no assets in an empty directory', async () => {
49-
server = await startAssetsServer(dir)
62+
server = await startAssetsServer(dir, { watch: false })
5063
client = bootClient(server.port)
5164
const list = await call(client, 'devframes:plugin:assets:list')
5265
expect(list).toEqual([])
5366
})
5467

5568
it('reports write capabilities by default', async () => {
56-
server = await startAssetsServer(dir)
69+
server = await startAssetsServer(dir, { watch: false })
5770
client = bootClient(server.port)
5871
const caps = await call(client, 'devframes:plugin:assets:capabilities')
5972
expect(caps).toEqual({ write: true, uploadExtensions: expect.any(Array) })
6073
})
6174

6275
it('creates a folder and uploads a file into it via the streaming channel', async () => {
63-
server = await startAssetsServer(dir)
76+
server = await startAssetsServer(dir, { watch: false })
6477
client = bootClient(server.port)
6578

6679
await call(client, 'devframes:plugin:assets:mkdir', { path: 'icons' })
@@ -73,7 +86,7 @@ describe('assets plugin', () => {
7386
})
7487

7588
it('reads image dimensions for an uploaded image', async () => {
76-
server = await startAssetsServer(dir)
89+
server = await startAssetsServer(dir, { watch: false })
7790
client = bootClient(server.port)
7891
await upload(client, 'logo.png', ONE_PIXEL_PNG)
7992

@@ -83,7 +96,7 @@ describe('assets plugin', () => {
8396

8497
it('reads text content for a text asset', async () => {
8598
await fsp.writeFile(join(dir, 'notes.txt'), 'hello world', 'utf-8')
86-
server = await startAssetsServer(dir)
99+
server = await startAssetsServer(dir, { watch: false })
87100
client = bootClient(server.port)
88101

89102
const content = await call(client, 'devframes:plugin:assets:read-text', 'notes.txt')
@@ -93,7 +106,7 @@ describe('assets plugin', () => {
93106
it('renames an asset within its folder', async () => {
94107
await fsp.mkdir(join(dir, 'icons'), { recursive: true })
95108
await fsp.writeFile(join(dir, 'icons/old.txt'), 'x', 'utf-8')
96-
server = await startAssetsServer(dir)
109+
server = await startAssetsServer(dir, { watch: false })
97110
client = bootClient(server.port)
98111

99112
const renamed = await call(client, 'devframes:plugin:assets:rename', { path: 'icons/old.txt', newName: 'new' })
@@ -105,7 +118,7 @@ describe('assets plugin', () => {
105118
it('rejects renaming onto an existing file', async () => {
106119
await fsp.writeFile(join(dir, 'a.txt'), 'a', 'utf-8')
107120
await fsp.writeFile(join(dir, 'b.txt'), 'b', 'utf-8')
108-
server = await startAssetsServer(dir)
121+
server = await startAssetsServer(dir, { watch: false })
109122
client = bootClient(server.port)
110123

111124
await expect(call(client, 'devframes:plugin:assets:rename', { path: 'a.txt', newName: 'b' })).rejects.toThrow()
@@ -114,7 +127,7 @@ describe('assets plugin', () => {
114127
it('deletes one or more assets in a single call', async () => {
115128
await fsp.writeFile(join(dir, 'a.txt'), 'a', 'utf-8')
116129
await fsp.writeFile(join(dir, 'b.txt'), 'b', 'utf-8')
117-
server = await startAssetsServer(dir)
130+
server = await startAssetsServer(dir, { watch: false })
118131
client = bootClient(server.port)
119132

120133
const result = await call(client, 'devframes:plugin:assets:delete', { paths: ['a.txt', 'b.txt', 'missing.txt'] })
@@ -123,7 +136,7 @@ describe('assets plugin', () => {
123136
})
124137

125138
it('rejects paths that escape the managed directory', async () => {
126-
server = await startAssetsServer(dir)
139+
server = await startAssetsServer(dir, { watch: false })
127140
client = bootClient(server.port)
128141

129142
await expect(call(client, 'devframes:plugin:assets:read-text', '../../etc/passwd')).resolves.toBeNull()
@@ -133,7 +146,7 @@ describe('assets plugin', () => {
133146
})
134147

135148
it('rejects uploads with a disallowed extension', async () => {
136-
server = await startAssetsServer(dir, { uploadExtensions: ['png'] })
149+
server = await startAssetsServer(dir, { uploadExtensions: ['png'], watch: false })
137150
client = bootClient(server.port)
138151

139152
await expect(
@@ -142,7 +155,7 @@ describe('assets plugin', () => {
142155
})
143156

144157
it('does not register write actions when write is disabled', async () => {
145-
server = await startAssetsServer(dir, { write: false })
158+
server = await startAssetsServer(dir, { write: false, watch: false })
146159
client = bootClient(server.port)
147160

148161
const caps = await call(client, 'devframes:plugin:assets:capabilities')
@@ -151,7 +164,7 @@ describe('assets plugin', () => {
151164
})
152165

153166
it('still registers open-in-editor and reveal-in-folder when write is disabled', async () => {
154-
server = await startAssetsServer(dir, { write: false })
167+
server = await startAssetsServer(dir, { write: false, watch: false })
155168

156169
// open-in-editor / reveal-in-folder launch external OS apps, so assert
157170
// they're *registered* on the server context rather than invoking them
@@ -163,6 +176,9 @@ describe('assets plugin', () => {
163176
expect(defs.has('devframes:plugin:assets:mkdir')).toBe(false)
164177
})
165178

179+
// The one test that needs the live watcher — every other test above opts
180+
// out of it (`watch: false`) so this is the only native fs watch handle
181+
// this file ever opens, close()s, and deletes the directory of.
166182
it('broadcasts a change event when a file is added on disk', async () => {
167183
server = await startAssetsServer(dir)
168184
client = bootClient(server.port)

0 commit comments

Comments
 (0)