diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 84c97318..becb7f88 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -19,15 +19,27 @@ env: jobs: test: - # Skip for Dependabot PRs — the LLM-dependent E2E tests require Azure OpenAI - # secrets that are not available in Dependabot PR runs. Full E2E coverage runs - # on the merge-to-staging/main push event where secrets are accessible. - if: github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]' timeout-minutes: 60 runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + + # Detect whether AI secrets are available (not available for Dependabot / fork PRs) + - name: Check for AI secrets + id: check-secrets + run: | + if [ -n "$AZURE_API_KEY" ] && [ -n "$AZURE_API_BASE" ] && [ -n "$AZURE_API_VERSION" ]; then + echo "has-ai-secrets=true" >> "$GITHUB_OUTPUT" + echo "AI secrets are available — full E2E suite will run" + else + echo "has-ai-secrets=false" >> "$GITHUB_OUTPUT" + echo "AI secrets are NOT available — only non-AI E2E tests will run" + fi + env: + AZURE_API_KEY: ${{ secrets.AZURE_API_KEY }} + AZURE_API_BASE: ${{ secrets.AZURE_API_BASE }} + AZURE_API_VERSION: ${{ secrets.AZURE_API_VERSION }} # Setup Python - name: Set up Python @@ -77,8 +89,9 @@ jobs: run: npm run build working-directory: ./app - # Start Docker Compose services (test databases) + # Start Docker Compose services (test databases) — only needed for AI tests - name: Start test databases + if: steps.check-secrets.outputs.has-ai-secrets == 'true' run: | docker compose -f e2e/docker-compose.test.yml up -d --wait --wait-timeout 120 docker ps -a @@ -173,9 +186,17 @@ jobs: - name: Create auth directory run: mkdir -p e2e/.auth - # Run Playwright tests - - name: Run Playwright tests - run: npx playwright test --reporter=list + # Run non-AI Playwright tests (always — no LLM secrets needed) + - name: Run Playwright tests (no AI) + run: npx playwright test --grep-invert @requires-ai --reporter=list + env: + CI: true + + # Run AI-dependent Playwright tests (only when secrets are available) + # Use !cancelled() so AI tests still run even if non-AI tests fail + - name: Run Playwright tests (AI) + if: "!cancelled() && steps.check-secrets.outputs.has-ai-secrets == 'true'" + run: npx playwright test --grep @requires-ai --reporter=list env: CI: true diff --git a/AGENTS.md b/AGENTS.md index e99aae12..4e63d2d3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,6 +116,7 @@ make build-prod # Vite production build - **Unit tests** (`tests/`): pytest with markers `e2e`, `slow`, `auth`, `integration`, `unit` - **E2E tests** (`e2e/`): Playwright with Page Object Model pattern; auth setup runs first +- E2E tests tagged `@requires-ai` need LLM secrets (Azure OpenAI) for DB schema loading; non-AI tests run unconditionally in CI - E2E infra lives in `e2e/infra/`, page objects in `e2e/logic/pom/` - Test data (SQL init scripts) in `e2e/test-data/` @@ -135,7 +136,7 @@ See `.env.example` for the full list. GitHub Actions workflows (`.github/workflows/`): - **tests.yml** — unit tests + lint on push/PR to main/staging -- **playwright.yml** — dedicated Playwright E2E suite (skipped for Dependabot PRs; secrets unavailable) +- **playwright.yml** — dedicated Playwright E2E suite; non-AI tests always run, `@requires-ai` tests only when LLM secrets are available (secrets are unavailable for Dependabot and fork PRs) - **pylint.yml** — Python linting - **spellcheck.yml** — docs spellcheck - **publish-docker.yml** — build & push Docker image to DockerHub diff --git a/e2e/tests/chat.spec.ts b/e2e/tests/chat.spec.ts index bbc78ecb..75b4fea0 100644 --- a/e2e/tests/chat.spec.ts +++ b/e2e/tests/chat.spec.ts @@ -41,7 +41,7 @@ test.describe('Chat Feature Tests', () => { expect(hasCorrectDescription).toBeTruthy(); }); - test('valid query shows SQL, results, and AI response', async () => { + test('valid query shows SQL, results, and AI response', { tag: '@requires-ai' }, async () => { const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -73,7 +73,7 @@ test.describe('Chat Feature Tests', () => { expect(finalAIMessageCount).toBeGreaterThanOrEqual(2); // At least welcome + final response }); - test('off-topic query shows AI message without SQL or results', async () => { + test('off-topic query shows AI message without SQL or results', { tag: '@requires-ai' }, async () => { const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -104,7 +104,7 @@ test.describe('Chat Feature Tests', () => { expect(aiText.length).toBeGreaterThan(0); }); - test('multiple sequential queries maintain conversation history', async () => { + test('multiple sequential queries maintain conversation history', { tag: '@requires-ai' }, async () => { test.slow(); // Two sequential LLM round-trips need extra time in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -145,15 +145,12 @@ test.describe('Chat Feature Tests', () => { const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); - // Ensure database is connected (will skip if already connected) - await homePage.ensureDatabaseConnected(apiCall); - - // Verify send button is disabled with empty input + // Verify send button is disabled with empty input (pure UI validation, no DB needed) const isSendButtonDisabled = await homePage.isSendQueryButtonDisabled(); expect(isSendButtonDisabled).toBeTruthy(); }); - test('rapid query submission is prevented during processing', async () => { + test('rapid query submission is prevented during processing', { tag: '@requires-ai' }, async () => { const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -171,7 +168,7 @@ test.describe('Chat Feature Tests', () => { expect(isDisabledDuringProcessing).toBeTruthy(); }); - test('switching databases clears chat history', async () => { + test('switching databases clears chat history', { tag: '@requires-ai' }, async () => { test.slow(); // Two database connections plus LLM round-trip need extra time in CI // Connect two databases via API const { postgres: postgresUrl } = getTestDatabases(); @@ -225,7 +222,7 @@ test.describe('Chat Feature Tests', () => { expect(aiMessageCount).toBe(1); }); - test('destructive operation shows inline confirmation and executes on confirm', async () => { + test('destructive operation shows inline confirmation and executes on confirm', { tag: '@requires-ai' }, async () => { const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -267,7 +264,7 @@ test.describe('Chat Feature Tests', () => { expect(finalAIMessageCount).toBeGreaterThan(1); // Welcome message + execution result }); - test('duplicate record shows user-friendly error message', async () => { + test('duplicate record shows user-friendly error message', { tag: '@requires-ai' }, async () => { test.slow(); // Two LLM round-trips with confirmation dialogs need extra time in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); diff --git a/e2e/tests/database.spec.ts b/e2e/tests/database.spec.ts index 73c9ab4c..62b81dbf 100644 --- a/e2e/tests/database.spec.ts +++ b/e2e/tests/database.spec.ts @@ -19,7 +19,7 @@ test.describe('Database Connection Tests', () => { await browser.closeBrowser(); }); - test('connect PostgreSQL via API -> verify in UI', async () => { + test('connect PostgreSQL via API -> verify in UI', { tag: '@requires-ai' }, async () => { test.setTimeout(120000); // Allow extra time for schema loading in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -71,7 +71,7 @@ test.describe('Database Connection Tests', () => { expect(isDatabaseVisible).toBeTruthy(); }); - test('connect MySQL via API -> verify in UI', async () => { + test('connect MySQL via API -> verify in UI', { tag: '@requires-ai' }, async () => { test.setTimeout(120000); // Allow extra time for schema loading in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -123,7 +123,7 @@ test.describe('Database Connection Tests', () => { expect(isDatabaseVisible).toBeTruthy(); }); - test('connect PostgreSQL via UI (URL) -> verify via API', async () => { + test('connect PostgreSQL via UI (URL) -> verify via API', { tag: '@requires-ai' }, async () => { test.setTimeout(120000); // Allow extra time for schema loading in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -162,7 +162,7 @@ test.describe('Database Connection Tests', () => { expect(isConnected).toBeTruthy(); }); - test('connect MySQL via UI (URL) -> verify via API', async () => { + test('connect MySQL via UI (URL) -> verify via API', { tag: '@requires-ai' }, async () => { test.setTimeout(120000); // Allow extra time for schema loading in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -201,7 +201,7 @@ test.describe('Database Connection Tests', () => { expect(isConnected).toBeTruthy(); }); - test('connect PostgreSQL via UI (Manual Entry) -> verify via API', async () => { + test('connect PostgreSQL via UI (Manual Entry) -> verify via API', { tag: '@requires-ai' }, async () => { test.setTimeout(120000); // Allow extra time for schema loading in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -245,7 +245,7 @@ test.describe('Database Connection Tests', () => { expect(isConnected).toBeTruthy(); }); - test('connect MySQL via UI (Manual Entry) -> verify via API', async () => { + test('connect MySQL via UI (Manual Entry) -> verify via API', { tag: '@requires-ai' }, async () => { test.setTimeout(120000); // Allow extra time for schema loading in CI const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json'); await browser.setPageToFullScreen(); @@ -318,7 +318,7 @@ test.describe('Database Connection Tests', () => { // Delete tests run serially to avoid conflicts test.describe.serial('Database Deletion Tests', () => { - test('delete PostgreSQL database via UI -> verify removed via API', async () => { + test('delete PostgreSQL database via UI -> verify removed via API', { tag: '@requires-ai' }, async () => { test.setTimeout(180000); // Allow extra time: schema loading + UI interaction // Use the separate postgres delete container on port 5433 const postgresDeleteUrl = 'postgresql://postgres:postgres@localhost:5433/testdb_delete'; @@ -369,7 +369,7 @@ test.describe('Database Connection Tests', () => { expect(graphsList).not.toContain(graphId); }); - test('delete MySQL database via UI -> verify removed via API', async () => { + test('delete MySQL database via UI -> verify removed via API', { tag: '@requires-ai' }, async () => { test.setTimeout(180000); // Allow extra time: schema loading + UI interaction // Use the separate mysql delete container on port 3307 const mysqlDeleteUrl = 'mysql://root:password@localhost:3307/testdb_delete';