Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions src/utils/check-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,26 @@ export async function checkPlugins(): Promise<BundleParams> {
sourcemap: false,
};

for (const plugin of plugins) {
try {
const isEnabled = await plugin.detect();
if (isEnabled && plugin.bundleParams) {
Object.assign(params, plugin.bundleParams);
console.log(t('pluginDetected', { name: plugin.name }));
const results = await Promise.all(
plugins.map(async (plugin) => {
try {
const isEnabled = await plugin.detect();
return { isEnabled, error: null };
} catch (error) {
return { isEnabled: false, error };
}
} catch (err) {
console.warn(
t('pluginDetectionError', { name: plugin.name, error: err }),
);
}),
);

results.forEach(({ isEnabled, error }, index) => {
const plugin = plugins[index];
if (error) {
console.warn(t('pluginDetectionError', { name: plugin.name, error }));
} else if (isEnabled && plugin.bundleParams) {
Object.assign(params, plugin.bundleParams);
console.log(t('pluginDetected', { name: plugin.name }));
}
}
});

return params;
}
75 changes: 75 additions & 0 deletions tests/check-plugin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { describe, expect, mock, spyOn, test, beforeEach, afterEach } from 'bun:test';

// Mock dependencies before imports
mock.module('fs-extra', () => ({
default: {
access: async () => {},
},
}));

mock.module('i18next', () => ({
default: {
init: () => {},
t: (key: string) => key,
},
}));

import { checkPlugins } from '../src/utils/check-plugin';
import * as pluginConfig from '../src/utils/plugin-config';

describe('checkPlugins', () => {
test('should detect plugins concurrently (simulated)', async () => {
const mockPlugins = [
{
name: 'plugin1',
bundleParams: { p1: true },
detect: async () => {
await new Promise(resolve => setTimeout(resolve, 100));
return true;
}
},
{
name: 'plugin2',
bundleParams: { p2: true },
detect: async () => {
await new Promise(resolve => setTimeout(resolve, 100));
return true;
}
},
{
name: 'plugin3',
bundleParams: { p3: true },
detect: async () => {
await new Promise(resolve => setTimeout(resolve, 100));
return true;
}
}
];

// Replacing the plugins array in plugin-config
const originalPlugins = [...pluginConfig.plugins];
(pluginConfig.plugins as any).splice(0, pluginConfig.plugins.length, ...mockPlugins);

const start = Date.now();
const result = await checkPlugins();
const end = Date.now();
const duration = end - start;

console.log(`Duration with optimized implementation: ${duration}ms`);

expect(result).toEqual({
sentry: false, // default
sourcemap: false, // default
p1: true,
p2: true,
p3: true
} as any);

// Now it's concurrent, so we expect around 100ms.
// We'll allow some buffer, but it should definitely be less than 250ms.
expect(duration).toBeLessThan(250);

// Restore original plugins
(pluginConfig.plugins as any).splice(0, pluginConfig.plugins.length, ...originalPlugins);
Comment on lines +50 to +73
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Always restore pluginConfig.plugins via finally.

At Line 51, global test state is mutated. If an assertion fails before Line 73, cleanup is skipped and later tests may fail for the wrong reason.

Proposed fix
 const originalPlugins = [...pluginConfig.plugins];
 (pluginConfig.plugins as any).splice(0, pluginConfig.plugins.length, ...mockPlugins);

-const start = Date.now();
-const result = await checkPlugins();
-const end = Date.now();
-const duration = end - start;
-
-console.log(`Duration with optimized implementation: ${duration}ms`);
-
-expect(result).toEqual({
-  sentry: false, // default
-  sourcemap: false, // default
-  p1: true,
-  p2: true,
-  p3: true
-} as any);
-
-// Now it's concurrent, so we expect around 100ms.
-// We'll allow some buffer, but it should definitely be less than 250ms.
-expect(duration).toBeLessThan(250);
-
-// Restore original plugins
-(pluginConfig.plugins as any).splice(0, pluginConfig.plugins.length, ...originalPlugins);
+try {
+  const start = Date.now();
+  const result = await checkPlugins();
+  const end = Date.now();
+  const duration = end - start;
+
+  console.log(`Duration with optimized implementation: ${duration}ms`);
+
+  expect(result).toEqual({
+    sentry: false, // default
+    sourcemap: false, // default
+    p1: true,
+    p2: true,
+    p3: true
+  } as any);
+
+  // Now it's concurrent, so we expect around 100ms.
+  // We'll allow some buffer, but it should definitely be less than 250ms.
+  expect(duration).toBeLessThan(250);
+} finally {
+  (pluginConfig.plugins as any).splice(0, pluginConfig.plugins.length, ...originalPlugins);
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/check-plugin.test.ts` around lines 50 - 73, The test mutates
pluginConfig.plugins directly and only restores it at the end, so wrap the
mutation and the assertions in a try/finally: save originalPlugins, replace
pluginConfig.plugins with mockPlugins before running checkPlugins(), then in the
finally block always restore pluginConfig.plugins using the saved
originalPlugins (use the same splice restoration logic shown); ensure the
timing/assertion code (start/await checkPlugins()/end/expect(duration)...
expect(result)...) remains inside the try so cleanup always runs even on
failure.

});
});
Loading