fix(tests): Stabilize tests and document chokidar-inspired Node watcher architecture
This commit is contained in:
@@ -1,5 +1,13 @@
|
|||||||
# Changelog
|
# Changelog
|
||||||
|
|
||||||
|
## 2025-12-11 - 6.2.4 - fix(tests)
|
||||||
|
Stabilize tests and document chokidar-inspired Node watcher architecture
|
||||||
|
|
||||||
|
- test: add waitForFileEvent helper to wait for events for a specific file (reduces test flakiness)
|
||||||
|
- test: add small delays after unlink cleanup to account for atomic/temp-file debounce windows
|
||||||
|
- docs: expand readme.hints.md with a detailed Node watcher architecture section (DirEntry, Throttler, atomic write handling, closer registry, constants and config)
|
||||||
|
- docs: list updated test files and coverage scenarios (inode detection, atomic writes, stress tests)
|
||||||
|
|
||||||
## 2025-12-11 - 6.2.3 - fix(watcher.node)
|
## 2025-12-11 - 6.2.3 - fix(watcher.node)
|
||||||
Improve handling of temporary files from atomic editor writes in Node watcher
|
Improve handling of temporary files from atomic editor writes in Node watcher
|
||||||
|
|
||||||
|
|||||||
@@ -71,6 +71,57 @@ The `WriteStabilizer` class replaces chokidar's built-in write stabilization:
|
|||||||
- **Deno**: Works on all versions with `Deno.watchFs()`
|
- **Deno**: Works on all versions with `Deno.watchFs()`
|
||||||
- **Bun**: Uses Node.js compatibility layer
|
- **Bun**: Uses Node.js compatibility layer
|
||||||
|
|
||||||
|
### Architecture (v6.3.0+) - Chokidar-Inspired
|
||||||
|
|
||||||
|
The Node.js watcher has been refactored with elegant patterns inspired by [chokidar](https://github.com/paulmillr/chokidar):
|
||||||
|
|
||||||
|
**DirEntry Class:**
|
||||||
|
- Tracks directory contents with proper disposal
|
||||||
|
- Encapsulates file tracking and inode management
|
||||||
|
- `dispose()` method freezes object to catch use-after-cleanup bugs
|
||||||
|
|
||||||
|
**Throttler Pattern:**
|
||||||
|
- More sophisticated than simple debounce
|
||||||
|
- Tracks count of suppressed events
|
||||||
|
- Returns `false` if already throttled, `Throttler` object otherwise
|
||||||
|
- Used for change events to prevent duplicate emissions
|
||||||
|
|
||||||
|
**Atomic Write Handling:**
|
||||||
|
- Unlink events are queued with 100ms delay
|
||||||
|
- If add event arrives for same path within delay, unlink is cancelled
|
||||||
|
- Emits single `change` event instead of `unlink` + `add`
|
||||||
|
- Handles editor atomic saves elegantly
|
||||||
|
|
||||||
|
**Closer Registry:**
|
||||||
|
- Maps watch paths to cleanup functions
|
||||||
|
- Ensures proper resource cleanup on stop
|
||||||
|
- `addCloser()` / `runClosers()` pattern
|
||||||
|
|
||||||
|
**Event Constants Object:**
|
||||||
|
```typescript
|
||||||
|
const EV = {
|
||||||
|
ADD: 'add',
|
||||||
|
CHANGE: 'change',
|
||||||
|
UNLINK: 'unlink',
|
||||||
|
ADD_DIR: 'addDir',
|
||||||
|
UNLINK_DIR: 'unlinkDir',
|
||||||
|
READY: 'ready',
|
||||||
|
ERROR: 'error',
|
||||||
|
} as const;
|
||||||
|
```
|
||||||
|
|
||||||
|
**Configuration Constants:**
|
||||||
|
```typescript
|
||||||
|
const CONFIG = {
|
||||||
|
MAX_RETRIES: 3,
|
||||||
|
INITIAL_RESTART_DELAY: 1000,
|
||||||
|
MAX_RESTART_DELAY: 30000,
|
||||||
|
HEALTH_CHECK_INTERVAL: 30000,
|
||||||
|
ATOMIC_DELAY: 100,
|
||||||
|
TEMP_FILE_DELAY: 50,
|
||||||
|
} as const;
|
||||||
|
```
|
||||||
|
|
||||||
### Robustness Features (v6.1.0+)
|
### Robustness Features (v6.1.0+)
|
||||||
|
|
||||||
The Node.js watcher includes automatic recovery mechanisms based on learnings from [chokidar](https://github.com/paulmillr/chokidar) and known [fs.watch issues](https://github.com/nodejs/node/issues/47058):
|
The Node.js watcher includes automatic recovery mechanisms based on learnings from [chokidar](https://github.com/paulmillr/chokidar) and known [fs.watch issues](https://github.com/nodejs/node/issues/47058):
|
||||||
@@ -155,10 +206,20 @@ Example log output:
|
|||||||
pnpm test
|
pnpm test
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Test files:
|
||||||
|
- **test.basic.ts** - Core functionality (add, change, unlink events)
|
||||||
|
- **test.inode.ts** - Inode change detection, atomic writes
|
||||||
|
- **test.stress.ts** - Rapid modifications, many files, interleaved operations
|
||||||
|
|
||||||
Tests verify:
|
Tests verify:
|
||||||
- Creating Smartwatch instance
|
- Creating Smartwatch instance
|
||||||
- Adding glob patterns
|
- Adding glob patterns
|
||||||
- Receiving 'add' events for new files
|
- Receiving 'add', 'change', 'unlink' events
|
||||||
|
- Inode change detection (delete+recreate pattern)
|
||||||
|
- Atomic write pattern (temp file + rename)
|
||||||
|
- Rapid file modifications (debouncing)
|
||||||
|
- Many files created rapidly
|
||||||
|
- Interleaved add/change/delete operations
|
||||||
- Graceful shutdown
|
- Graceful shutdown
|
||||||
|
|
||||||
## Dev Dependencies
|
## Dev Dependencies
|
||||||
|
|||||||
@@ -35,6 +35,30 @@ async function waitForEvent<T>(
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Helper to wait for a specific file's event (filters by filename)
|
||||||
|
async function waitForFileEvent<T extends [string, ...any[]]>(
|
||||||
|
observable: smartrx.rxjs.Observable<T>,
|
||||||
|
expectedFile: string,
|
||||||
|
timeoutMs: number = 5000
|
||||||
|
): Promise<T> {
|
||||||
|
return new Promise((resolve, reject) => {
|
||||||
|
const timeout = setTimeout(() => {
|
||||||
|
subscription.unsubscribe();
|
||||||
|
reject(new Error(`Timeout waiting for event on ${expectedFile} after ${timeoutMs}ms`));
|
||||||
|
}, timeoutMs);
|
||||||
|
|
||||||
|
const subscription = observable.subscribe((value) => {
|
||||||
|
const [filePath] = value;
|
||||||
|
if (filePath.includes(expectedFile)) {
|
||||||
|
clearTimeout(timeout);
|
||||||
|
subscription.unsubscribe();
|
||||||
|
resolve(value);
|
||||||
|
}
|
||||||
|
// Otherwise keep waiting for the right file
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
let testSmartwatch: smartwatch.Smartwatch;
|
let testSmartwatch: smartwatch.Smartwatch;
|
||||||
|
|
||||||
// ===========================================
|
// ===========================================
|
||||||
@@ -63,8 +87,9 @@ tap.test('should detect ADD event for new files', async () => {
|
|||||||
const [filePath] = await eventPromise;
|
const [filePath] = await eventPromise;
|
||||||
expect(filePath).toInclude('add-test.txt');
|
expect(filePath).toInclude('add-test.txt');
|
||||||
|
|
||||||
// Cleanup
|
// Cleanup - wait for atomic delay to complete (100ms debounce + 100ms atomic)
|
||||||
await fs.promises.unlink(testFile);
|
await fs.promises.unlink(testFile);
|
||||||
|
await delay(250);
|
||||||
});
|
});
|
||||||
|
|
||||||
tap.test('should detect CHANGE event for modified files', async () => {
|
tap.test('should detect CHANGE event for modified files', async () => {
|
||||||
@@ -84,8 +109,9 @@ tap.test('should detect CHANGE event for modified files', async () => {
|
|||||||
const [filePath] = await eventPromise;
|
const [filePath] = await eventPromise;
|
||||||
expect(filePath).toInclude('change-test.txt');
|
expect(filePath).toInclude('change-test.txt');
|
||||||
|
|
||||||
// Cleanup
|
// Cleanup - wait for atomic delay to complete
|
||||||
await fs.promises.unlink(testFile);
|
await fs.promises.unlink(testFile);
|
||||||
|
await delay(250);
|
||||||
});
|
});
|
||||||
|
|
||||||
tap.test('should detect UNLINK event for deleted files', async () => {
|
tap.test('should detect UNLINK event for deleted files', async () => {
|
||||||
@@ -97,7 +123,9 @@ tap.test('should detect UNLINK event for deleted files', async () => {
|
|||||||
await delay(200);
|
await delay(200);
|
||||||
|
|
||||||
const unlinkObservable = await testSmartwatch.getObservableFor('unlink');
|
const unlinkObservable = await testSmartwatch.getObservableFor('unlink');
|
||||||
const eventPromise = waitForEvent(unlinkObservable);
|
|
||||||
|
// Use file-specific wait to handle any pending unlinks from other tests
|
||||||
|
const eventPromise = waitForFileEvent(unlinkObservable, 'unlink-test.txt');
|
||||||
|
|
||||||
// Delete the file
|
// Delete the file
|
||||||
await fs.promises.unlink(testFile);
|
await fs.promises.unlink(testFile);
|
||||||
|
|||||||
@@ -3,6 +3,6 @@
|
|||||||
*/
|
*/
|
||||||
export const commitinfo = {
|
export const commitinfo = {
|
||||||
name: '@push.rocks/smartwatch',
|
name: '@push.rocks/smartwatch',
|
||||||
version: '6.2.3',
|
version: '6.2.4',
|
||||||
description: 'A cross-runtime file watcher with glob pattern support for Node.js, Deno, and Bun.'
|
description: 'A cross-runtime file watcher with glob pattern support for Node.js, Deno, and Bun.'
|
||||||
}
|
}
|
||||||
|
|||||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user