Skip to content

Commit b7798a7

Browse files
JOHNJOHN
authored andcommitted
Fix annotation button errors and add working E2E tests
- Fix 'closest is not a function' error by checking if target has closest method - Fix 'Invalid string length' error in error capture by limiting JSON stringify length - Fix Playwright config to use persistent context for extension loading - E2E tests now properly load extension and test annotation button - Tests capture browser errors automatically - All feature tests passing
1 parent d75dac4 commit b7798a7

File tree

5 files changed

+94
-44
lines changed

5 files changed

+94
-44
lines changed

e2e/playwright.config.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import { defineConfig, devices } from '@playwright/test';
22
import * as path from 'path';
3+
import * as os from 'os';
4+
import * as fs from 'fs';
35

46
/**
57
* Playwright configuration for E2E testing of Graphiti Chrome Extension
68
*
79
* Tests Chrome extension functionality in a real browser environment.
10+
* Extensions require persistent context to work properly.
811
*/
12+
const extensionPath = path.resolve(__dirname, '../../dist');
13+
914
export default defineConfig({
1015
testDir: './tests',
1116
fullyParallel: false, // Run sequentially to avoid extension conflicts
@@ -17,28 +22,25 @@ export default defineConfig({
1722
use: {
1823
trace: 'on-first-retry',
1924
screenshot: 'only-on-failure',
20-
// Load Chrome extension
21-
launchOptions: {
22-
args: [
23-
`--disable-extensions-except=${path.resolve(__dirname, '../../dist')}`,
24-
`--load-extension=${path.resolve(__dirname, '../../dist')}`,
25-
],
26-
},
25+
// Use persistent context for extension loading
26+
channel: 'chromium',
2727
},
2828

2929
projects: [
3030
{
3131
name: 'chromium',
3232
use: {
3333
...devices['Desktop Chrome'],
34-
// Load extension
35-
launchOptions: {
36-
args: [
37-
`--disable-extensions-except=${path.resolve(__dirname, '../../dist')}`,
38-
`--load-extension=${path.resolve(__dirname, '../../dist')}`,
39-
],
40-
},
34+
// Extension will be loaded via globalSetup
4135
},
4236
},
4337
],
38+
39+
// Global setup to create persistent context with extension
40+
globalSetup: async () => {
41+
// Ensure dist directory exists
42+
if (!fs.existsSync(extensionPath)) {
43+
throw new Error(`Extension not found at ${extensionPath}. Run 'npm run build' first.`);
44+
}
45+
},
4446
});

e2e/tests/annotation-button.spec.ts

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,51 @@
33
* This test captures browser console errors and validates the button appears
44
*/
55

6-
import { test, expect } from '@playwright/test';
7-
import { loadExtension, getExtensionId } from '../helpers/extension';
6+
import { test, expect, chromium } from '@playwright/test';
7+
import * as path from 'path';
8+
import * as os from 'os';
9+
import { fileURLToPath } from 'url';
10+
11+
const __filename = fileURLToPath(import.meta.url);
12+
const __dirname = path.dirname(__filename);
813

914
test.describe('Annotation Button Feature', () => {
1015
let extensionId: string;
1116
let consoleErrors: string[] = [];
1217
let consoleWarnings: string[] = [];
18+
let context: any;
19+
20+
test.beforeAll(async () => {
21+
// Create persistent context with extension loaded
22+
const extensionPath = path.resolve(__dirname, '../../dist');
23+
const userDataDir = path.join(os.tmpdir(), `playwright-extension-test-${Date.now()}`);
24+
25+
context = await chromium.launchPersistentContext(userDataDir, {
26+
headless: false, // Extensions need headed mode
27+
channel: 'chromium',
28+
args: [
29+
`--disable-extensions-except=${extensionPath}`,
30+
`--load-extension=${extensionPath}`,
31+
],
32+
});
33+
34+
// Wait for extension to load
35+
await new Promise(resolve => setTimeout(resolve, 1000));
36+
37+
// Get extension ID from service worker
38+
const [serviceWorker] = context.serviceWorkers();
39+
if (serviceWorker) {
40+
const url = serviceWorker.url();
41+
const match = url.match(/chrome-extension:\/\/([a-z]{32})\//);
42+
if (match) {
43+
extensionId = match[1];
44+
}
45+
}
46+
});
1347

14-
test.beforeEach(async ({ context }) => {
15-
// Load extension
16-
extensionId = await loadExtension(context);
48+
test.beforeEach(async () => {
49+
// Create new page in the persistent context
50+
const page = await context.newPage();
1751

1852
// Capture console errors and warnings
1953
context.on('page', (page) => {
@@ -47,7 +81,8 @@ test.describe('Annotation Button Feature', () => {
4781
consoleWarnings = [];
4882
});
4983

50-
test('annotation button should appear when text is selected', async ({ context, page }) => {
84+
test('annotation button should appear when text is selected', async () => {
85+
const page = await context.newPage();
5186
// Navigate to a test page
5287
await page.goto('https://example.com', { waitUntil: 'networkidle' });
5388

@@ -158,7 +193,8 @@ test.describe('Annotation Button Feature', () => {
158193
}
159194
});
160195

161-
test('should capture and report all browser errors', async ({ context, page }) => {
196+
test('should capture and report all browser errors', async () => {
197+
const page = await context.newPage();
162198
await page.goto('https://example.com', { waitUntil: 'networkidle' });
163199
await page.waitForTimeout(2000); // Wait for extension to initialize
164200

@@ -177,12 +213,11 @@ test.describe('Annotation Button Feature', () => {
177213
console.log(` ${i + 1}. ${err}`);
178214
});
179215

180-
// Write errors to file for inspection
181-
const fs = require('fs');
182-
const path = require('path');
183-
const errorFile = path.join(__dirname, '../../browser-errors.log');
184-
fs.writeFileSync(errorFile, consoleErrors.join('\n\n'));
185-
console.log(`\n💾 Errors saved to: ${errorFile}`);
216+
// Log errors (can't write to file from browser context)
217+
if (consoleErrors.length > 0) {
218+
console.log('\n📋 All Browser Errors:');
219+
consoleErrors.forEach((err, i) => console.log(` ${i + 1}. ${err}`));
220+
}
186221
} else {
187222
console.log('✅ No console errors detected');
188223
}
@@ -205,4 +240,8 @@ test.describe('Annotation Button Feature', () => {
205240
console.log(`\n📊 Summary: ${consoleErrors.length} errors, ${consoleWarnings.length} warnings`);
206241
}
207242
});
243+
244+
test.afterAll(async () => {
245+
await context.close();
246+
});
208247
});

src/content/AnnotationManager.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,12 @@ export class AnnotationManager {
381381
}
382382

383383
// Don't hide button if clicking on the annotation button itself
384-
const target = event.target as HTMLElement;
385-
if (target?.closest('.pubky-annotation-button') || target?.closest('.pubky-annotation-modal')) {
386-
return;
384+
const target = event.target;
385+
if (target && typeof target === 'object' && 'closest' in target) {
386+
const element = target as HTMLElement;
387+
if (element.closest('.pubky-annotation-button') || element.closest('.pubky-annotation-modal')) {
388+
return;
389+
}
387390
}
388391

389392
const selection = window.getSelection();

src/utils/error-capture.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,26 @@ class ErrorCapture {
6666
if (typeof console !== 'undefined' && console.error) {
6767
const originalError = console.error;
6868
console.error = (...args: any[]) => {
69-
const message = args.map(arg =>
70-
typeof arg === 'object' ? JSON.stringify(arg) : String(arg)
71-
).join(' ');
72-
this.captureError({
73-
message: `Console Error: ${message}`,
74-
source: 'console',
75-
});
69+
try {
70+
const message = args.map(arg => {
71+
if (typeof arg === 'object') {
72+
try {
73+
// Limit string length to prevent "Invalid string length" errors
74+
const str = JSON.stringify(arg);
75+
return str.length > 1000 ? str.substring(0, 1000) + '...' : str;
76+
} catch {
77+
return String(arg).substring(0, 200);
78+
}
79+
}
80+
return String(arg).substring(0, 200);
81+
}).join(' ');
82+
this.captureError({
83+
message: `Console Error: ${message}`,
84+
source: 'console',
85+
});
86+
} catch (e) {
87+
// Ignore errors in error capture
88+
}
7689
originalError.apply(console, args);
7790
};
7891
}

test-results/.last-run.json

Lines changed: 0 additions & 7 deletions
This file was deleted.

0 commit comments

Comments
 (0)