From 4894253e48a7057a7b2b0120248211fc35448c63 Mon Sep 17 00:00:00 2001 From: Juergen Kunz Date: Wed, 10 Dec 2025 16:52:06 +0000 Subject: [PATCH] fix(watcher.node): Handle fs.watch close without spurious restarts; add tests and improve test runner --- changelog.md | 8 ++ package.json | 2 +- test/assets/hi.txt | 1 - test/test.basic.ts | 127 ++++++++++++++++++++++++++ test/test.inode.ts | 129 ++++++++++++++++++++++++++ test/test.stress.ts | 174 ++++++++++++++++++++++++++++++++++++ test/test.ts | 50 ----------- ts/00_commitinfo_data.ts | 2 +- ts/watchers/watcher.node.ts | 3 +- 9 files changed, 442 insertions(+), 54 deletions(-) delete mode 100644 test/assets/hi.txt create mode 100644 test/test.basic.ts create mode 100644 test/test.inode.ts create mode 100644 test/test.stress.ts delete mode 100644 test/test.ts diff --git a/changelog.md b/changelog.md index 1768613..c5f5a47 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,13 @@ # Changelog +## 2025-12-10 - 6.2.1 - fix(watcher.node) +Handle fs.watch close without spurious restarts; add tests and improve test runner + +- Prevent spurious restarts and noisy warnings on fs.watch 'close' by checking the internal isWatching flag before logging and restarting (ts/watchers/watcher.node.ts). +- Add comprehensive test suites covering basic operations, inode-change detection, atomic writes and stress scenarios (test/test.basic.ts, test/test.inode.ts, test/test.stress.ts). +- Remove outdated test (test/test.ts) and delete the test asset test/assets/hi.txt. +- Update test script in package.json to enable verbose logging, write a logfile and increase timeout to 120s to reduce flakiness in test runs. + ## 2025-12-10 - 6.2.0 - feat(watchers) Improve Node watcher robustness: file-level inode tracking, abortable restarts, restart race guards, and untracked-file handling diff --git a/package.json b/package.json index bcd690b..ef7eb00 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "main": "dist_ts/index.js", "typings": "dist_ts/index.d.ts", "scripts": { - "test": "(npm run prepareTest && tstest test/)", + "test": "(npm run prepareTest && tstest test/ --verbose --logfile --timeout 120)", "prepareTest": "(rm -f ./test/assets/hi.txt)", "build": "tsbuild tsfolders", "buildDocs": "tsdoc" diff --git a/test/assets/hi.txt b/test/assets/hi.txt deleted file mode 100644 index 56f44d3..0000000 --- a/test/assets/hi.txt +++ /dev/null @@ -1 +0,0 @@ -HI \ No newline at end of file diff --git a/test/test.basic.ts b/test/test.basic.ts new file mode 100644 index 0000000..018864a --- /dev/null +++ b/test/test.basic.ts @@ -0,0 +1,127 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import * as smartwatch from '../ts/index.js'; +import * as smartfile from '@push.rocks/smartfile'; +import * as smartrx from '@push.rocks/smartrx'; + +import * as fs from 'fs'; +import * as path from 'path'; + +// Skip in CI +if (process.env.CI) { + process.exit(0); +} + +const TEST_DIR = './test/assets'; + +// Helper to delay +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +// Helper to wait for an event with timeout +async function waitForEvent( + observable: smartrx.rxjs.Observable, + timeoutMs: number = 5000 +): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + subscription.unsubscribe(); + reject(new Error(`Timeout waiting for event after ${timeoutMs}ms`)); + }, timeoutMs); + + const subscription = observable.subscribe((value) => { + clearTimeout(timeout); + subscription.unsubscribe(); + resolve(value); + }); + }); +} + +let testSmartwatch: smartwatch.Smartwatch; + +// =========================================== +// BASIC TESTS +// =========================================== + +tap.test('should create a new instance', async () => { + testSmartwatch = new smartwatch.Smartwatch([]); + expect(testSmartwatch).toBeInstanceOf(smartwatch.Smartwatch); +}); + +tap.test('should add paths and start watching', async () => { + testSmartwatch.add([`${TEST_DIR}/**/*.txt`]); + await testSmartwatch.start(); + expect(testSmartwatch.status).toEqual('watching'); +}); + +tap.test('should detect ADD event for new files', async () => { + const addObservable = await testSmartwatch.getObservableFor('add'); + const eventPromise = waitForEvent(addObservable); + + // Create a new file + const testFile = path.join(TEST_DIR, 'add-test.txt'); + await smartfile.memory.toFs('test content', testFile); + + const [filePath] = await eventPromise; + expect(filePath).toInclude('add-test.txt'); + + // Cleanup + await fs.promises.unlink(testFile); +}); + +tap.test('should detect CHANGE event for modified files', async () => { + // First create the file + const testFile = path.join(TEST_DIR, 'change-test.txt'); + await smartfile.memory.toFs('initial content', testFile); + + // Wait for add event to complete + await delay(200); + + const changeObservable = await testSmartwatch.getObservableFor('change'); + const eventPromise = waitForEvent(changeObservable); + + // Modify the file + await smartfile.memory.toFs('modified content', testFile); + + const [filePath] = await eventPromise; + expect(filePath).toInclude('change-test.txt'); + + // Cleanup + await fs.promises.unlink(testFile); +}); + +tap.test('should detect UNLINK event for deleted files', async () => { + // First create the file + const testFile = path.join(TEST_DIR, 'unlink-test.txt'); + await smartfile.memory.toFs('to be deleted', testFile); + + // Wait for add event to complete + await delay(200); + + const unlinkObservable = await testSmartwatch.getObservableFor('unlink'); + const eventPromise = waitForEvent(unlinkObservable); + + // Delete the file + await fs.promises.unlink(testFile); + + const [filePath] = await eventPromise; + expect(filePath).toInclude('unlink-test.txt'); +}); + +tap.test('should stop the watch process', async () => { + await testSmartwatch.stop(); + expect(testSmartwatch.status).toEqual('idle'); +}); + +tap.test('cleanup: remove any remaining test files', async () => { + const files = await fs.promises.readdir(TEST_DIR); + for (const file of files) { + if (file.startsWith('add-') || file.startsWith('change-') || file.startsWith('unlink-')) { + try { + await fs.promises.unlink(path.join(TEST_DIR, file)); + } catch { + // Ignore + } + } + } +}); + +export default tap.start(); diff --git a/test/test.inode.ts b/test/test.inode.ts new file mode 100644 index 0000000..7fb6943 --- /dev/null +++ b/test/test.inode.ts @@ -0,0 +1,129 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import * as smartwatch from '../ts/index.js'; +import * as smartfile from '@push.rocks/smartfile'; +import * as smartrx from '@push.rocks/smartrx'; + +import * as fs from 'fs'; +import * as path from 'path'; + +// Skip in CI +if (process.env.CI) { + process.exit(0); +} + +const TEST_DIR = './test/assets'; + +// Helper to delay +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +// Helper to wait for an event with timeout +async function waitForEvent( + observable: smartrx.rxjs.Observable, + timeoutMs: number = 5000 +): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + subscription.unsubscribe(); + reject(new Error(`Timeout waiting for event after ${timeoutMs}ms`)); + }, timeoutMs); + + const subscription = observable.subscribe((value) => { + clearTimeout(timeout); + subscription.unsubscribe(); + resolve(value); + }); + }); +} + +let testSmartwatch: smartwatch.Smartwatch; + +// =========================================== +// INODE CHANGE DETECTION TESTS +// =========================================== + +tap.test('setup: start watcher', async () => { + testSmartwatch = new smartwatch.Smartwatch([`${TEST_DIR}/**/*.txt`]); + await testSmartwatch.start(); + expect(testSmartwatch.status).toEqual('watching'); +}); + +tap.test('should detect delete+recreate (inode change scenario)', async () => { + // This simulates what many editors do: delete file, create new file + const testFile = path.join(TEST_DIR, 'inode-test.txt'); + + // Create initial file + await smartfile.memory.toFs('initial content', testFile); + await delay(200); + + // Get the initial inode + const initialStats = await fs.promises.stat(testFile); + const initialInode = initialStats.ino; + console.log(`[test] Initial inode: ${initialInode}`); + + const changeObservable = await testSmartwatch.getObservableFor('change'); + const eventPromise = waitForEvent(changeObservable, 3000); + + // Delete and recreate (this creates a new inode) + await fs.promises.unlink(testFile); + await smartfile.memory.toFs('recreated content', testFile); + + // Check inode changed + const newStats = await fs.promises.stat(testFile); + const newInode = newStats.ino; + console.log(`[test] New inode: ${newInode}`); + expect(newInode).not.toEqual(initialInode); + + // Should still detect the change + const [filePath] = await eventPromise; + expect(filePath).toInclude('inode-test.txt'); + + // Cleanup + await fs.promises.unlink(testFile); +}); + +tap.test('should detect atomic write pattern (temp file + rename)', async () => { + // This simulates what Claude Code and many editors do: + // 1. Write to temp file (file.txt.tmp.12345) + // 2. Rename temp file to target file + const testFile = path.join(TEST_DIR, 'atomic-test.txt'); + const tempFile = path.join(TEST_DIR, 'atomic-test.txt.tmp.12345'); + + // Create initial file + await smartfile.memory.toFs('initial content', testFile); + await delay(200); + + const changeObservable = await testSmartwatch.getObservableFor('change'); + const eventPromise = waitForEvent(changeObservable, 3000); + + // Atomic write: create temp file then rename + await smartfile.memory.toFs('atomic content', tempFile); + await fs.promises.rename(tempFile, testFile); + + // Should detect the change to the target file + const [filePath] = await eventPromise; + expect(filePath).toInclude('atomic-test.txt'); + expect(filePath).not.toInclude('.tmp.'); + + // Cleanup + await fs.promises.unlink(testFile); +}); + +tap.test('teardown: stop watcher', async () => { + await testSmartwatch.stop(); + expect(testSmartwatch.status).toEqual('idle'); +}); + +tap.test('cleanup: remove test files', async () => { + const files = await fs.promises.readdir(TEST_DIR); + for (const file of files) { + if (file.startsWith('inode-') || file.startsWith('atomic-')) { + try { + await fs.promises.unlink(path.join(TEST_DIR, file)); + } catch { + // Ignore + } + } + } +}); + +export default tap.start(); diff --git a/test/test.stress.ts b/test/test.stress.ts new file mode 100644 index 0000000..742e283 --- /dev/null +++ b/test/test.stress.ts @@ -0,0 +1,174 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import * as smartwatch from '../ts/index.js'; +import * as smartfile from '@push.rocks/smartfile'; +import * as smartrx from '@push.rocks/smartrx'; + +import * as fs from 'fs'; +import * as path from 'path'; + +// Skip in CI +if (process.env.CI) { + process.exit(0); +} + +const TEST_DIR = './test/assets'; + +// Helper to delay +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +// Helper to collect events +function collectEvents( + observable: smartrx.rxjs.Observable, + durationMs: number +): Promise { + return new Promise((resolve) => { + const events: T[] = []; + const subscription = observable.subscribe((value) => { + events.push(value); + }); + + setTimeout(() => { + subscription.unsubscribe(); + resolve(events); + }, durationMs); + }); +} + +let testSmartwatch: smartwatch.Smartwatch; + +// =========================================== +// STRESS TESTS +// =========================================== + +tap.test('setup: start watcher', async () => { + testSmartwatch = new smartwatch.Smartwatch([`${TEST_DIR}/**/*.txt`]); + await testSmartwatch.start(); + expect(testSmartwatch.status).toEqual('watching'); +}); + +tap.test('STRESS: rapid file modifications', async () => { + const testFile = path.join(TEST_DIR, 'stress-rapid.txt'); + + // Create initial file + await smartfile.memory.toFs('initial', testFile); + await delay(200); + + const changeObservable = await testSmartwatch.getObservableFor('change'); + + // Rapidly modify the file 20 times + const RAPID_CHANGES = 20; + const eventCollector = collectEvents(changeObservable, 3000); + + for (let i = 0; i < RAPID_CHANGES; i++) { + await smartfile.memory.toFs(`content ${i}`, testFile); + await delay(10); // 10ms between writes + } + + const events = await eventCollector; + console.log(`[test] Rapid changes: sent ${RAPID_CHANGES}, received ${events.length} events`); + + // Due to debouncing, we won't get all events, but we should get at least some + expect(events.length).toBeGreaterThan(0); + + // Cleanup + await fs.promises.unlink(testFile); +}); + +tap.test('STRESS: many files created rapidly', async () => { + const FILE_COUNT = 20; + const files: string[] = []; + + const addObservable = await testSmartwatch.getObservableFor('add'); + const eventCollector = collectEvents(addObservable, 5000); + + // Create many files rapidly + for (let i = 0; i < FILE_COUNT; i++) { + const file = path.join(TEST_DIR, `stress-many-${i}.txt`); + files.push(file); + await smartfile.memory.toFs(`content ${i}`, file); + await delay(20); // 20ms between creates + } + + const events = await eventCollector; + console.log(`[test] Many files: created ${FILE_COUNT}, detected ${events.length} events`); + + // Should detect most or all files + expect(events.length).toBeGreaterThanOrEqual(FILE_COUNT * 0.8); // Allow 20% tolerance + + // Cleanup all files + for (const file of files) { + try { + await fs.promises.unlink(file); + } catch { + // Ignore if already deleted + } + } +}); + +tap.test('STRESS: interleaved add/change/delete operations', async () => { + const testFiles = [ + path.join(TEST_DIR, 'stress-interleave-1.txt'), + path.join(TEST_DIR, 'stress-interleave-2.txt'), + path.join(TEST_DIR, 'stress-interleave-3.txt'), + ]; + + // Create initial files + for (const file of testFiles) { + await smartfile.memory.toFs('initial', file); + } + await delay(300); + + const addObservable = await testSmartwatch.getObservableFor('add'); + const changeObservable = await testSmartwatch.getObservableFor('change'); + const unlinkObservable = await testSmartwatch.getObservableFor('unlink'); + + const addEvents = collectEvents(addObservable, 3000); + const changeEvents = collectEvents(changeObservable, 3000); + const unlinkEvents = collectEvents(unlinkObservable, 3000); + + // Interleaved operations + await smartfile.memory.toFs('changed 1', testFiles[0]); // change + await delay(50); + await fs.promises.unlink(testFiles[1]); // delete + await delay(50); + await smartfile.memory.toFs('recreated 1', testFiles[1]); // add (recreate) + await delay(50); + await smartfile.memory.toFs('changed 2', testFiles[2]); // change + await delay(50); + + const [adds, changes, unlinks] = await Promise.all([addEvents, changeEvents, unlinkEvents]); + + console.log(`[test] Interleaved: adds=${adds.length}, changes=${changes.length}, unlinks=${unlinks.length}`); + + // Should have detected some events of each type + expect(changes.length).toBeGreaterThan(0); + + // Cleanup + for (const file of testFiles) { + try { + await fs.promises.unlink(file); + } catch { + // Ignore + } + } +}); + +tap.test('teardown: stop watcher', async () => { + await testSmartwatch.stop(); + expect(testSmartwatch.status).toEqual('idle'); +}); + +tap.test('cleanup: remove stress test files', async () => { + const files = await fs.promises.readdir(TEST_DIR); + for (const file of files) { + if (file.startsWith('stress-')) { + try { + await fs.promises.unlink(path.join(TEST_DIR, file)); + } catch { + // Ignore + } + } + } +}); + +export default tap.start(); diff --git a/test/test.ts b/test/test.ts deleted file mode 100644 index 07beeec..0000000 --- a/test/test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { tap, expect } from '@git.zone/tstest/tapbundle'; -import * as smartwatch from '../ts/index.js'; -import * as smartfile from '@push.rocks/smartfile'; -import * as smartpromise from '@push.rocks/smartpromise'; -import * as smartrx from '@push.rocks/smartrx'; - -import * as fs from 'fs'; - -// the module to test -if (process.env.CI) { - process.exit(0); -} - -let testSmartwatch: smartwatch.Smartwatch; -let testAddObservable: smartrx.rxjs.Observable<[string, fs.Stats]>; -let testSubscription: smartrx.rxjs.Subscription; -tap.test('should create a new instance', async () => { - testSmartwatch = new smartwatch.Smartwatch([]); - expect(testSmartwatch).toBeInstanceOf(smartwatch.Smartwatch); -}); - -tap.test('should add some files to watch and start', async () => { - testSmartwatch.add(['./test/**/*.txt']); - await testSmartwatch.start() - testSmartwatch.add(['./test/**/*.md']); -}); - -tap.test('should get an observable for a certain event', async () => { - await testSmartwatch.getObservableFor('add').then(async (observableArg) => { - testAddObservable = observableArg; - }); -}); - -tap.test('should register an add operation', async () => { - let testDeferred = smartpromise.defer(); - testSubscription = testAddObservable.subscribe(pathArg => { - const pathResult = pathArg[0]; - console.log(pathResult); - testDeferred.resolve(); - }); - smartfile.memory.toFs('HI', './test/assets/hi.txt'); - await testDeferred.promise; -}); - -tap.test('should stop the watch process', async (tools) => { - await tools.delayFor(10000); - testSmartwatch.stop(); -}); - -export default tap.start(); diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index efda168..46f05e3 100644 --- a/ts/00_commitinfo_data.ts +++ b/ts/00_commitinfo_data.ts @@ -3,6 +3,6 @@ */ export const commitinfo = { name: '@push.rocks/smartwatch', - version: '6.2.0', + version: '6.2.1', description: 'A cross-runtime file watcher with glob pattern support for Node.js, Deno, and Bun.' } diff --git a/ts/watchers/watcher.node.ts b/ts/watchers/watcher.node.ts index 8bf5e48..8ee4e56 100644 --- a/ts/watchers/watcher.node.ts +++ b/ts/watchers/watcher.node.ts @@ -339,8 +339,9 @@ export class NodeWatcher implements IWatcher { // Handle 'close' event - fs.watch can close without error watcher.on('close', () => { - console.warn(`[smartwatch] FSWatcher closed unexpectedly for ${watchPath}`); + // Only log/restart if we didn't intentionally stop if (this._isWatching) { + console.warn(`[smartwatch] FSWatcher closed unexpectedly for ${watchPath}`); this.restartWatcher(watchPath, new Error('Watcher closed unexpectedly')); } });