test: add more test cases for improving coverage in plugin engine

rudderlabs · Jan 10, 2025 · 0ecb545 · 0ecb545
1 parent ef841ae
commit 0ecb545
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 3 deletions.
diff --git a/packages/analytics-js/__tests__/services/PluginEngine/PluginEngine.test.ts b/packages/analytics-js/__tests__/services/PluginEngine/PluginEngine.test.ts
@@ -1,10 +1,11 @@
 import type { ExtensionPlugin } from '@rudderstack/analytics-js-common/types/PluginEngine';
+import { defaultLogger } from '@rudderstack/analytics-js-common/__mocks__/Logger';
 import { PluginEngine } from '../../../src/services/PluginEngine/PluginEngine';
-import { defaultLogger } from '../../../src/services/Logger';
 
 const mockPlugin1: ExtensionPlugin = {
   name: 'p1',
   foo: 'bar1',
+  initialize: jest.fn(),
   ext: {
     form: {
       processMeta(meta: string[]) {
@@ -41,8 +42,6 @@ describe('PluginEngine', () => {
     pluginEngineTestInstance.register(mockPlugin3);
   });
 
-  afterEach(() => {});
-
   it('should retrieve all registered plugins', () => {
     expect(pluginEngineTestInstance.getPlugins().length).toEqual(3);
   });
@@ -56,6 +55,40 @@ describe('PluginEngine', () => {
     expect(pluginEngineTestInstance.getPlugins().length).toEqual(4);
   });
 
+  it('should throw error for missing plugin name if configured', () => {
+    // @ts-expect-error Testing for missing name
+    expect(() => {
+      pluginEngineTestInstance.register({});
+    }).toThrow(new Error('PluginEngine:: Plugin name is missing.'));
+  });
+
+  it('should log an error for missing plugin name', () => {
+    // Temporarily mutate the config
+    pluginEngineTestInstance.config.throws = false;
+
+    // @ts-expect-error Testing for missing name
+    pluginEngineTestInstance.register({});
+
+    expect(defaultLogger.error).toHaveBeenCalledTimes(1);
+    expect(defaultLogger.error).toHaveBeenCalledWith('PluginEngine:: Plugin name is missing.', {});
+  });
+
+  it('should throw error for already registered plugin name if configured', () => {
+    expect(() => {
+      pluginEngineTestInstance.register({ name: 'p1' });
+    }).toThrow(new Error('PluginEngine:: Plugin "p1" already exists.'));
+  });
+
+  it('should log an error for already registered plugin name', () => {
+    // Temporarily mutate the config
+    pluginEngineTestInstance.config.throws = false;
+
+    pluginEngineTestInstance.register({ name: 'p1' });
+
+    expect(defaultLogger.error).toHaveBeenCalledTimes(1);
+    expect(defaultLogger.error).toHaveBeenCalledWith('PluginEngine:: Plugin "p1" already exists.');
+  });
+
   it('should invoke multiple plugins on functions', () => {
     const meta = ['m0'];
     pluginEngineTestInstance.invokeMultiple('ext.form.processMeta', meta);
@@ -90,6 +123,50 @@ describe('PluginEngine', () => {
     expect(pluginEngineTestInstance.getPlugin('p2')).toBeUndefined();
   });
 
+  it('should throw an error if the plugin to unregister does not exist', () => {
+    expect(() => {
+      pluginEngineTestInstance.unregister('p0');
+    }).toThrow(new Error('PluginEngine:: Plugin "p0" not found.'));
+  });
+
+  it('should log an error if the plugin to unregister does not exist', () => {
+    // Temporarily mutate the config
+    pluginEngineTestInstance.config.throws = false;
+
+    pluginEngineTestInstance.unregister('p0');
+
+    expect(defaultLogger.error).toHaveBeenCalledTimes(1);
+    expect(defaultLogger.error).toHaveBeenCalledWith('PluginEngine:: Plugin "p0" not found.');
+  });
+
+  it('should throw an error if the plugin to unregister is found in byName but already registered', () => {
+    // Temporarily mutate the plugins array
+    pluginEngineTestInstance.plugins = [mockPlugin2, mockPlugin3];
+
+    expect(() => {
+      pluginEngineTestInstance.unregister('p1');
+    }).toThrow(
+      new Error(
+        'PluginEngine:: Plugin "p1" not found in plugins but found in byName. This indicates a bug in the plugin engine. Please report this issue to the development team.',
+      ),
+    );
+  });
+
+  it('should log an error if the plugin to unregister is found in byName but already registered', () => {
+    // Temporarily mutate the plugins array
+    pluginEngineTestInstance.plugins = [mockPlugin2, mockPlugin3];
+
+    // Temporarily mutate the config
+    pluginEngineTestInstance.config.throws = false;
+
+    pluginEngineTestInstance.unregister('p1');
+
+    expect(defaultLogger.error).toHaveBeenCalledTimes(1);
+    expect(defaultLogger.error).toHaveBeenCalledWith(
+      'PluginEngine:: Plugin "p1" not found in plugins but found in byName. This indicates a bug in the plugin engine. Please report this issue to the development team.',
+    );
+  });
+
   it('should not load if deps do not exist', () => {
     pluginEngineTestInstance.register({ name: 'p4', deps: ['p5'] });
     expect(pluginEngineTestInstance.getPlugins().map(p => p.name)).toStrictEqual([
@@ -195,6 +272,21 @@ describe('PluginEngine', () => {
     pluginEngineTestInstance.invokeMultiple('fail');
   });
 
+  it('should throw an error if extension point is not provided', () => {
+    expect(() => {
+      pluginEngineTestInstance.invoke();
+    }).toThrow(new Error('Failed to invoke plugin because the extension point name is missing.'));
+  });
+
+  it('should throw an error if extension point is invalid', () => {
+    // Temporarily mutate the config
+    pluginEngineTestInstance.config.throws = undefined;
+
+    expect(() => {
+      pluginEngineTestInstance.invoke('!');
+    }).toThrow(new Error('Failed to invoke plugin because the extension point name is invalid.'));
+  });
+
   it('should register 1000 plugins in less than 200ms', () => {
     const time1 = Date.now();
     for (let i = 0; i < 1000; i++) {

diff --git a/packages/analytics-js/src/services/PluginEngine/PluginEngine.ts b/packages/analytics-js/src/services/PluginEngine/PluginEngine.ts
@@ -47,6 +47,7 @@ class PluginEngine implements IPluginEngine {
         throw new Error(errorMessage);
       } else {
         this.logger.error(errorMessage, plugin);
+        return;
       }
     }
 
@@ -56,6 +57,7 @@ class PluginEngine implements IPluginEngine {
         throw new Error(errorMessage);
       } else {
         this.logger.error(errorMessage);
+        return;
       }
     }
 
@@ -87,6 +89,7 @@ class PluginEngine implements IPluginEngine {
         throw new Error(errorMessage);
       } else {
         this.logger.error(errorMessage);
+        return;
       }
     }
 
@@ -98,6 +101,7 @@ class PluginEngine implements IPluginEngine {
         throw new Error(errorMessage);
       } else {
         this.logger.error(errorMessage);
+        return;
       }
     }