fix: pr feedback 2

Author: Kyle Walker

.github/workflows/e2e-tests.yml

@@ -28,50 +28,32 @@ jobs:
 
       - name: Build all packages and extension
         run: |
-          echo "🔧 Building all packages..."
           npm run compile
           npm run bundle
-          echo "🔧 Building VS Code extension..."
-          cd packages/apex-lsp-vscode-extension
-          npm run build
-          echo "✅ Build completed"
-          ls -la dist/
+          npm run build -w packages/apex-lsp-vscode-extension
 
       - name: Verify extension build
         run: |
-          echo "🔍 Verifying extension build artifacts..."
-          cd packages/apex-lsp-vscode-extension
-          if [ ! -f "dist/package.json" ]; then
+          if [ ! -f "packages/apex-lsp-vscode-extension/dist/package.json" ]; then
             echo "❌ package.json not found in dist"
             exit 1
           fi
-          if [ ! -f "dist/extension.js" ]; then
+          if [ ! -f "packages/apex-lsp-vscode-extension/dist/extension.js" ]; then
             echo "❌ extension.js not found in dist"
             exit 1
           fi
-          if [ ! -f "dist/extension.web.js" ]; then
+          if [ ! -f "packages/apex-lsp-vscode-extension/dist/extension.web.js" ]; then
             echo "❌ extension.web.js not found in dist"
             exit 1
           fi
-          echo "✅ All required extension files present"
 
       - name: Install Playwright browsers
         run: npx playwright install --with-deps chromium
 
       - name: Run E2E tests
-        run: |
-          echo "🧪 Starting e2e tests..."
-          echo "Environment variables:"
-          echo "  CI: $CI"
-          echo "  RUNNER_TEMP: $RUNNER_TEMP"
-          echo "  TMPDIR: $TMPDIR"
-          echo "  PWD: $PWD"
-          echo "Test workspace path: $RUNNER_TEMP/apex-e2e-workspace"
-          echo "Running Playwright tests with proper CI configuration..."
-          cd e2e-tests && npx playwright test
+        run: npm run test -w e2e-tests
         env:
           CI: true
-          DEBUG: pw:webserver
 
       - name: Upload test results and artifacts
         if: always()
@@ -83,17 +65,3 @@ jobs:
             e2e-tests/test-results/
           retention-days: 30
           if-no-files-found: warn
-
-      - name: Debug artifact directories
-        if: always()
-        run: |
-          echo "🔍 Debugging artifact directories..."
-          echo "Current working directory: $(pwd)"
-          echo "e2e-tests directory contents:"
-          ls -la e2e-tests/ || echo "e2e-tests directory not found"
-          echo "playwright-report directory:"
-          ls -la e2e-tests/playwright-report/ || echo "playwright-report directory not found"
-          echo "test-results directory:"
-          ls -la e2e-tests/test-results/ || echo "test-results directory not found"
-          echo "Searching for any Playwright artifacts..."
-          find . -name "*.png" -o -name "*.webm" -o -name "*.html" | head -20

e2e-tests/package.json

@@ -0,0 +1,17 @@
+{
+  "name": "e2e-tests",
+  "version": "1.0.0",
+  "private": true,
+  "description": "End-to-end tests for Apex Language Server Extension",
+  "scripts": {
+    "test": "playwright test",
+    "test:debug": "playwright test --debug --headed",
+    "test:visual": "DEBUG_MODE=1 playwright test --headed",
+    "server": "node test-server.js"
+  },
+  "devDependencies": {
+    "@playwright/test": "^1.55.0",
+    "@types/node": "^20.11.30",
+    "typescript": "^5.8.2"
+  }
+}

e2e-tests/tests/apex-extension-core.spec.ts

@@ -15,11 +15,9 @@ import {
   activateExtension,
   waitForLSPInitialization,
   verifyVSCodeStability,
-  filterCriticalErrors,
-  reportTestResults,
+  validateAllErrorsInAllowList,
+  validateAllNetworkErrorsInAllowList,
   verifyApexFileContentLoaded,
-  logStep,
-  logSuccess,
 } from '../utils/test-helpers';
 
 import { setupTestWorkspace } from '../utils/setup';
@@ -28,10 +26,9 @@ import {
   findAndActivateOutlineView,
   validateApexSymbolsInOutline,
   captureOutlineViewScreenshot,
-  EXPECTED_APEX_SYMBOLS,
 } from '../utils/outline-helpers';
 
-import { ASSERTION_THRESHOLDS, SELECTORS } from '../utils/constants';
+import { SELECTORS, EXPECTED_APEX_SYMBOLS } from '../utils/constants';
 
 /**
  * Core E2E tests for Apex Language Server Extension.
@@ -65,31 +62,49 @@ test.describe('Apex Extension Core Functionality', () => {
 
     // Set up monitoring using utilities
     const consoleErrors = setupConsoleMonitoring(page);
-    const networkFailures = setupNetworkMonitoring(page);
+    const networkErrors = setupNetworkMonitoring(page);
 
     // Execute test steps using helper functions
     await startVSCodeWeb(page);
     const fileCount = await verifyWorkspaceFiles(page);
     await activateExtension(page);
     await waitForLSPInitialization(page);
 
-    // Filter and analyze errors
-    const criticalErrors = filterCriticalErrors(consoleErrors);
+    // Validate all errors are in allowList (strict validation)
+    const errorValidation = validateAllErrorsInAllowList(consoleErrors);
 
     // Report findings
-    if (criticalErrors.length > 0) {
-      console.log(
-        '⚠️  Critical console errors found:',
-        criticalErrors.map((e) => `${e.text} (${e.url})`),
-      );
+    console.log('📊 Console error validation:');
+    console.log(`   - Total errors: ${errorValidation.totalErrors}`);
+    console.log(`   - Allowed errors: ${errorValidation.allowedErrors}`);
+    console.log(
+      `   - Non-allowed errors: ${errorValidation.nonAllowedErrors.length}`,
+    );
+
+    if (!errorValidation.allErrorsAllowed) {
+      console.log('❌ NON-ALLOWED console errors found:');
+      errorValidation.nonAllowedErrors.forEach((error, index) => {
+        console.log(
+          `  ${index + 1}. "${error.text}" (URL: ${error.url || 'no URL'})`,
+        );
+      });
     } else {
-      console.log('✅ No critical console errors');
+      console.log('✅ All console errors are in allowList');
     }
 
-    if (networkFailures.length > 0) {
-      console.log('⚠️  Worker network failures:', networkFailures);
+    // Validate all network errors are in allowList (strict validation)
+    const networkValidation =
+      validateAllNetworkErrorsInAllowList(networkErrors);
+
+    if (!networkValidation.allErrorsAllowed) {
+      console.log('❌ NON-ALLOWED network errors found:');
+      networkValidation.nonAllowedErrors.forEach((error, index) => {
+        console.log(
+          `  ${index + 1}. HTTP ${error.status} ${error.url} (${error.description})`,
+        );
+      });
     } else {
-      console.log('✅ No worker loading failures');
+      console.log('✅ All network errors are in allowList');
     }
 
     // Verify extension in extensions list
@@ -107,21 +122,14 @@ test.describe('Apex Extension Core Functionality', () => {
     // Final stability verification
     await verifyVSCodeStability(page);
 
-    // Assert success criteria using constants
-    expect(criticalErrors.length).toBeLessThan(
-      ASSERTION_THRESHOLDS.MAX_CRITICAL_ERRORS,
-    );
-    expect(networkFailures.length).toBeLessThan(
-      ASSERTION_THRESHOLDS.MAX_NETWORK_FAILURES,
-    );
-    expect(fileCount).toBeGreaterThan(ASSERTION_THRESHOLDS.MIN_FILE_COUNT);
+    // Assert success criteria - STRICT validation: all errors must be in allowList
+    expect(errorValidation.allErrorsAllowed).toBe(true);
+    expect(networkValidation.allErrorsAllowed).toBe(true);
+    expect(fileCount).toBeGreaterThan(0); // find at least one Apex file
 
     // Report final results
-    reportTestResults(
-      'Core functionality',
-      fileCount,
-      criticalErrors.length,
-      networkFailures.length,
+    console.log(
+      `🎉 Core functionality test PASSED - ${fileCount} Apex files loaded, all errors validated`,
     );
   });
 
@@ -166,10 +174,18 @@ test.describe('Apex Extension Core Functionality', () => {
     await findAndActivateOutlineView(page);
 
     // Validate that specific Apex symbols are populated in the outline
-    const symbolValidation = await validateApexSymbolsInOutline(page);
+    const symbolValidation = await validateApexSymbolsInOutline(
+      page,
+      EXPECTED_APEX_SYMBOLS,
+    );
+
+    // Assert exact matches instead of loose counting
+    expect(symbolValidation.classFound).toBe(true);
+    expect(symbolValidation.allExpectedMethodsFound).toBe(true);
+    expect(symbolValidation.exactMatch).toBe(true);
 
     // Additionally check for complex symbols that may be present in the comprehensive class
-    logStep('Validating comprehensive symbol hierarchy', '🏗️');
+    console.log('🏗️ Validating comprehensive symbol hierarchy...');
 
     // Expected additional symbols in ApexClassExample.cls (beyond the basic ones)
     const additionalSymbols = [
@@ -201,13 +217,13 @@ test.describe('Apex Extension Core Functionality', () => {
           additionalSymbolsFound++;
           foundAdditionalSymbols.push(symbol);
           symbolFound = true;
-          logSuccess(`Found additional symbol: ${symbol}`);
+          console.log(`✅ Found additional symbol: ${symbol}`);
           break;
         }
       }
 
       if (!symbolFound) {
-        logStep(`Additional symbol not found: ${symbol}`, '⚪');
+        console.log(`⚪ Additional symbol not found: ${symbol}`);
       }
     }
 
@@ -217,53 +233,57 @@ test.describe('Apex Extension Core Functionality', () => {
     );
     const totalItems = await outlineItems.count();
 
-    // Filter and analyze errors
-    const criticalErrors = filterCriticalErrors(consoleErrors);
+    // Validate all errors are in allowList (strict validation)
+    const errorValidation = validateAllErrorsInAllowList(consoleErrors);
 
-    if (criticalErrors.length > 0) {
-      console.log(
-        '⚠️  Critical console errors found:',
-        criticalErrors.map((e) => `${e.text} (${e.url})`),
-      );
+    // Report findings
+    console.log('📊 Outline test - Console error validation:');
+    console.log(`   - Total errors: ${errorValidation.totalErrors}`);
+    console.log(`   - Allowed errors: ${errorValidation.allowedErrors}`);
+    console.log(
+      `   - Non-allowed errors: ${errorValidation.nonAllowedErrors.length}`,
+    );
+
+    if (!errorValidation.allErrorsAllowed) {
+      console.log('❌ NON-ALLOWED console errors found:');
+      errorValidation.nonAllowedErrors.forEach((error, index) => {
+        console.log(
+          `  ${index + 1}. "${error.text}" (URL: ${error.url || 'no URL'})`,
+        );
+      });
     } else {
-      console.log('✅ No critical console errors');
+      console.log('✅ All console errors are in allowList');
     }
 
     // Capture screenshot for debugging
     await captureOutlineViewScreenshot(page, 'comprehensive-outline-test.png');
 
-    // Assert comprehensive success criteria
-    expect(criticalErrors.length).toBeLessThan(
-      ASSERTION_THRESHOLDS.MAX_CRITICAL_ERRORS,
-    );
-    expect(symbolValidation.classFound).toBe(true);
-    expect(symbolValidation.methodsFound.length).toBeGreaterThanOrEqual(
-      EXPECTED_APEX_SYMBOLS.methods.length,
-    );
-    expect(symbolValidation.isValidStructure).toBe(true);
-    expect(symbolValidation.totalSymbolsDetected).toBeGreaterThan(0);
+    // Assert comprehensive success criteria - STRICT validation with exact matching
+    expect(errorValidation.allErrorsAllowed).toBe(true);
+    expect(symbolValidation.exactMatch).toBe(true);
+    expect(symbolValidation.missingMethods).toHaveLength(0);
     expect(totalItems).toBeGreaterThan(0);
 
-    // Verify specific methods are found
-    for (const method of EXPECTED_APEX_SYMBOLS.methods) {
-      expect(symbolValidation.methodsFound).toContain(method.name);
-    }
+    // Verify all specific methods are found (exact matching)
+    expect(symbolValidation.exactMethodsFound).toEqual(
+      expect.arrayContaining(EXPECTED_APEX_SYMBOLS.methods.map((m) => m.name)),
+    );
 
     // Report comprehensive results combining both basic and advanced symbol detection
     console.log('🎉 Comprehensive outline view test COMPLETED');
     console.log('   - File: ✅ ApexClassExample.cls opened and loaded');
     console.log('   - Extension: ✅ Language features activated');
     console.log('   - Outline: ✅ Outline view loaded and accessible');
 
-    // Basic symbols (required)
+    // Basic symbols (required) - exact matching
     console.log('   - Basic symbols: ✅ All expected symbols found');
     console.log(
-      `     • Class: ${symbolValidation.classFound ? '✅' : '❌'} ApexClassExample`,
+      `     • Class: ${symbolValidation.classFound ? '✅' : '❌'} ${EXPECTED_APEX_SYMBOLS.className}`,
     );
     console.log(
-      `     • Methods: ${symbolValidation.methodsFound.length}/${
+      `     • Methods: ${symbolValidation.exactMethodsFound.length}/${
         EXPECTED_APEX_SYMBOLS.methods.length
-      } (${symbolValidation.methodsFound.join(', ')})`,
+      } (${symbolValidation.exactMethodsFound.join(', ')})`,
     );
 
     // Additional symbols (nice to have)
@@ -276,9 +296,7 @@ test.describe('Apex Extension Core Functionality', () => {
 
     console.log(`   - Total outline elements: ${totalItems}`);
     console.log(
-      `   - Errors: ✅ ${criticalErrors.length} critical errors (threshold: ${
-        ASSERTION_THRESHOLDS.MAX_CRITICAL_ERRORS
-      })`,
+      `   - Errors: ✅ ${errorValidation.nonAllowedErrors.length} non-allowed errors (strict validation: must be 0)`,
     );
     console.log(
       '   ✨ This test validates comprehensive LSP symbol parsing and outline population',

e2e-tests/tsconfig.json

@@ -1,9 +1,9 @@
 {
   "compilerOptions": {
-    "target": "ES2020",
-    "lib": ["ES2020", "DOM"],
-    "module": "CommonJS",
-    "moduleResolution": "node",
+    "target": "ES2023",
+    "lib": ["ES2023", "DOM"],
+    "module": "nodenext",
+    "moduleResolution": "nodenext",
     "esModuleInterop": true,
     "allowSyntheticDefaultImports": true,
     "strict": true,
@@ -15,11 +15,6 @@
     "rootDir": ".",
     "types": ["node", "@playwright/test"]
   },
-  "include": [
-    "**/*.ts"
-  ],
-  "exclude": [
-    "node_modules",
-    "dist"
-  ]
-}
+  "include": ["**/*.ts"],
+  "exclude": ["node_modules", "dist"]
+}