refactor: remove cleanup function registration from HookManager and related tests

This commit is contained in:
sanish-bruno
2026-01-29 14:51:13 +05:30
parent 63e5fbab36
commit 5b9511c50d
3 changed files with 5 additions and 60 deletions

View File

@@ -58,8 +58,6 @@ class HookManager {
constructor() {
this.listeners = {};
// Track cleanup functions for proper resource disposal
this._cleanupFunctions = [];
this._state = HookManager.State.ACTIVE;
}
@@ -79,20 +77,9 @@ class HookManager {
return this._state === HookManager.State.DISPOSED;
}
/**
* Register a cleanup function to be called when dispose() is invoked
* This is used to clean up VM handles, handler references, etc.
* @param {Function} cleanupFn - Cleanup function to register
*/
registerCleanup(cleanupFn) {
if (typeof cleanupFn === 'function') {
this._cleanupFunctions.push(cleanupFn);
}
}
/**
* Dispose of all resources held by this HookManager
* Calls all registered cleanup functions and clears all handlers
* Clears all handlers and marks the manager as disposed
* Should be called when the HookManager is no longer needed
*/
dispose() {
@@ -101,16 +88,6 @@ class HookManager {
}
this._state = HookManager.State.DISPOSED;
// Call all registered cleanup functions
for (const cleanupFn of this._cleanupFunctions) {
try {
cleanupFn();
} catch (e) {
// Ignore cleanup errors - resources may already be freed
}
}
this._cleanupFunctions = [];
// Clear all listeners
this.clearAll();
}

View File

@@ -129,7 +129,6 @@ class HooksRuntime {
// Execute hooks script
// Note: Hooks need the VM to persist so registered handlers can be called later
// The cleanup function is registered with the HookManager and called when dispose() is invoked
if (this.runtime === 'nodevm') {
await runScriptInNodeVm({
script: hooksFile,
@@ -154,18 +153,12 @@ class HooksRuntime {
}
// For QuickJS, persist the VM so hook handlers can be called later during the collection run
// The cleanup function is registered with the HookManager to be called when dispose() is invoked
const result = await executeQuickJsVmAsync({
await executeQuickJsVmAsync({
script: hooksFile,
context: context,
collectionPath
});
// Register VM cleanup with HookManager so it's disposed when HookManager.dispose() is called
if (result?.cleanup && typeof activeHookManager.registerCleanup === 'function') {
activeHookManager.registerCleanup(result.cleanup);
}
return {
hookManager: activeHookManager,
envVariables: cleanJson(envVariables),

View File

@@ -215,13 +215,6 @@ describe('HookManager', () => {
expect(hookManager.state).toBe(HookManager.State.DISPOSED);
});
it('should call cleanup functions', () => {
const cleanup = jest.fn();
hookManager.registerCleanup(cleanup);
hookManager.dispose();
expect(cleanup).toHaveBeenCalled();
});
it('should clear all listeners', () => {
hookManager.on('test', jest.fn());
hookManager.dispose();
@@ -229,29 +222,11 @@ describe('HookManager', () => {
});
it('should be idempotent', () => {
const cleanup = jest.fn();
hookManager.registerCleanup(cleanup);
hookManager.on('test', jest.fn());
hookManager.dispose();
hookManager.dispose();
expect(cleanup).toHaveBeenCalledTimes(1);
});
it('should handle cleanup errors gracefully', () => {
hookManager.registerCleanup(() => { throw new Error('Cleanup error'); });
expect(() => hookManager.dispose()).not.toThrow();
});
});
describe('registerCleanup()', () => {
it('should register cleanup function', () => {
const cleanup = jest.fn();
hookManager.registerCleanup(cleanup);
hookManager.dispose();
expect(cleanup).toHaveBeenCalled();
});
it('should ignore non-function arguments', () => {
expect(() => hookManager.registerCleanup('not a function')).not.toThrow();
expect(hookManager.isDisposed).toBe(true);
expect(hookManager.listeners).toEqual({});
});
});
});