Skip to content

Commit 73609f3

Browse files
gkorlandCopilot
andcommitted
ci(playwright): split E2E tests into AI and non-AI groups
Tag tests requiring LLM secrets (Azure OpenAI) with @requires-ai. The workflow now runs in two phases: - Non-AI tests (sidebar, userProfile, error paths) always run - AI tests (database connection, chat queries) only run when secrets are available This replaces the previous blanket skip for Dependabot PRs with a granular approach that retains smoke test coverage even when secrets are unavailable. Docker Compose test databases are also conditionally started only when AI tests will run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bdc533f commit 73609f3

4 files changed

Lines changed: 44 additions & 25 deletions

File tree

.github/workflows/playwright.yml

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,25 @@ env:
1919

2020
jobs:
2121
test:
22-
# Skip for Dependabot PRs — the LLM-dependent E2E tests require Azure OpenAI
23-
# secrets that are not available in Dependabot PR runs. Full E2E coverage runs
24-
# on the merge-to-staging/main push event where secrets are accessible.
25-
if: github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]'
2622
timeout-minutes: 60
2723
runs-on: ubuntu-latest
2824

2925
steps:
3026
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
27+
28+
# Detect whether AI secrets are available (not available for Dependabot / fork PRs)
29+
- name: Check for AI secrets
30+
id: check-secrets
31+
run: |
32+
if [ -n "$AZURE_API_KEY" ]; then
33+
echo "has-ai-secrets=true" >> "$GITHUB_OUTPUT"
34+
echo "AI secrets are available — full E2E suite will run"
35+
else
36+
echo "has-ai-secrets=false" >> "$GITHUB_OUTPUT"
37+
echo "AI secrets are NOT available — only non-AI E2E tests will run"
38+
fi
39+
env:
40+
AZURE_API_KEY: ${{ secrets.AZURE_API_KEY }}
3141

3242
# Setup Python
3343
- name: Set up Python
@@ -77,8 +87,9 @@ jobs:
7787
run: npm run build
7888
working-directory: ./app
7989

80-
# Start Docker Compose services (test databases)
90+
# Start Docker Compose services (test databases) — only needed for AI tests
8191
- name: Start test databases
92+
if: steps.check-secrets.outputs.has-ai-secrets == 'true'
8293
run: |
8394
docker compose -f e2e/docker-compose.test.yml up -d --wait --wait-timeout 120
8495
docker ps -a
@@ -173,9 +184,16 @@ jobs:
173184
- name: Create auth directory
174185
run: mkdir -p e2e/.auth
175186

176-
# Run Playwright tests
177-
- name: Run Playwright tests
178-
run: npx playwright test --reporter=list
187+
# Run non-AI Playwright tests (always — no LLM secrets needed)
188+
- name: Run Playwright tests (no AI)
189+
run: npx playwright test --grep-invert @requires-ai --reporter=list
190+
env:
191+
CI: true
192+
193+
# Run AI-dependent Playwright tests (only when secrets are available)
194+
- name: Run Playwright tests (AI)
195+
if: steps.check-secrets.outputs.has-ai-secrets == 'true'
196+
run: npx playwright test --grep @requires-ai --reporter=list
179197
env:
180198
CI: true
181199

AGENTS.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ make build-prod # Vite production build
116116

117117
- **Unit tests** (`tests/`): pytest with markers `e2e`, `slow`, `auth`, `integration`, `unit`
118118
- **E2E tests** (`e2e/`): Playwright with Page Object Model pattern; auth setup runs first
119+
- E2E tests tagged `@requires-ai` need LLM secrets (Azure OpenAI) for DB schema loading; non-AI tests run unconditionally in CI
119120
- E2E infra lives in `e2e/infra/`, page objects in `e2e/logic/pom/`
120121
- Test data (SQL init scripts) in `e2e/test-data/`
121122

@@ -135,7 +136,7 @@ See `.env.example` for the full list.
135136

136137
GitHub Actions workflows (`.github/workflows/`):
137138
- **tests.yml** — unit tests + lint on push/PR to main/staging
138-
- **playwright.yml** — dedicated Playwright E2E suite (skipped for Dependabot PRs; secrets unavailable)
139+
- **playwright.yml** — dedicated Playwright E2E suite; non-AI tests always run, `@requires-ai` tests only when LLM secrets are available
139140
- **pylint.yml** — Python linting
140141
- **spellcheck.yml** — docs spellcheck
141142
- **publish-docker.yml** — build & push Docker image to DockerHub

e2e/tests/chat.spec.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ test.describe('Chat Feature Tests', () => {
4141
expect(hasCorrectDescription).toBeTruthy();
4242
});
4343

44-
test('valid query shows SQL, results, and AI response', async () => {
44+
test('valid query shows SQL, results, and AI response', { tag: '@requires-ai' }, async () => {
4545
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
4646
await browser.setPageToFullScreen();
4747

@@ -73,7 +73,7 @@ test.describe('Chat Feature Tests', () => {
7373
expect(finalAIMessageCount).toBeGreaterThanOrEqual(2); // At least welcome + final response
7474
});
7575

76-
test('off-topic query shows AI message without SQL or results', async () => {
76+
test('off-topic query shows AI message without SQL or results', { tag: '@requires-ai' }, async () => {
7777
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
7878
await browser.setPageToFullScreen();
7979

@@ -104,7 +104,7 @@ test.describe('Chat Feature Tests', () => {
104104
expect(aiText.length).toBeGreaterThan(0);
105105
});
106106

107-
test('multiple sequential queries maintain conversation history', async () => {
107+
test('multiple sequential queries maintain conversation history', { tag: '@requires-ai' }, async () => {
108108
test.slow(); // Two sequential LLM round-trips need extra time in CI
109109
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
110110
await browser.setPageToFullScreen();
@@ -141,7 +141,7 @@ test.describe('Chat Feature Tests', () => {
141141
expect(resultsVisible).toBeTruthy();
142142
});
143143

144-
test('empty query submission is prevented', async () => {
144+
test('empty query submission is prevented', { tag: '@requires-ai' }, async () => {
145145
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
146146
await browser.setPageToFullScreen();
147147

@@ -153,7 +153,7 @@ test.describe('Chat Feature Tests', () => {
153153
expect(isSendButtonDisabled).toBeTruthy();
154154
});
155155

156-
test('rapid query submission is prevented during processing', async () => {
156+
test('rapid query submission is prevented during processing', { tag: '@requires-ai' }, async () => {
157157
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
158158
await browser.setPageToFullScreen();
159159

@@ -171,7 +171,7 @@ test.describe('Chat Feature Tests', () => {
171171
expect(isDisabledDuringProcessing).toBeTruthy();
172172
});
173173

174-
test('switching databases clears chat history', async () => {
174+
test('switching databases clears chat history', { tag: '@requires-ai' }, async () => {
175175
test.slow(); // Two database connections plus LLM round-trip need extra time in CI
176176
// Connect two databases via API
177177
const { postgres: postgresUrl } = getTestDatabases();
@@ -225,7 +225,7 @@ test.describe('Chat Feature Tests', () => {
225225
expect(aiMessageCount).toBe(1);
226226
});
227227

228-
test('destructive operation shows inline confirmation and executes on confirm', async () => {
228+
test('destructive operation shows inline confirmation and executes on confirm', { tag: '@requires-ai' }, async () => {
229229
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
230230
await browser.setPageToFullScreen();
231231

@@ -267,7 +267,7 @@ test.describe('Chat Feature Tests', () => {
267267
expect(finalAIMessageCount).toBeGreaterThan(1); // Welcome message + execution result
268268
});
269269

270-
test('duplicate record shows user-friendly error message', async () => {
270+
test('duplicate record shows user-friendly error message', { tag: '@requires-ai' }, async () => {
271271
test.slow(); // Two LLM round-trips with confirmation dialogs need extra time in CI
272272
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
273273
await browser.setPageToFullScreen();

e2e/tests/database.spec.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ test.describe('Database Connection Tests', () => {
1919
await browser.closeBrowser();
2020
});
2121

22-
test('connect PostgreSQL via API -> verify in UI', async () => {
22+
test('connect PostgreSQL via API -> verify in UI', { tag: '@requires-ai' }, async () => {
2323
test.setTimeout(120000); // Allow extra time for schema loading in CI
2424
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
2525
await browser.setPageToFullScreen();
@@ -71,7 +71,7 @@ test.describe('Database Connection Tests', () => {
7171
expect(isDatabaseVisible).toBeTruthy();
7272
});
7373

74-
test('connect MySQL via API -> verify in UI', async () => {
74+
test('connect MySQL via API -> verify in UI', { tag: '@requires-ai' }, async () => {
7575
test.setTimeout(120000); // Allow extra time for schema loading in CI
7676
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
7777
await browser.setPageToFullScreen();
@@ -123,7 +123,7 @@ test.describe('Database Connection Tests', () => {
123123
expect(isDatabaseVisible).toBeTruthy();
124124
});
125125

126-
test('connect PostgreSQL via UI (URL) -> verify via API', async () => {
126+
test('connect PostgreSQL via UI (URL) -> verify via API', { tag: '@requires-ai' }, async () => {
127127
test.setTimeout(120000); // Allow extra time for schema loading in CI
128128
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
129129
await browser.setPageToFullScreen();
@@ -162,7 +162,7 @@ test.describe('Database Connection Tests', () => {
162162
expect(isConnected).toBeTruthy();
163163
});
164164

165-
test('connect MySQL via UI (URL) -> verify via API', async () => {
165+
test('connect MySQL via UI (URL) -> verify via API', { tag: '@requires-ai' }, async () => {
166166
test.setTimeout(120000); // Allow extra time for schema loading in CI
167167
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
168168
await browser.setPageToFullScreen();
@@ -201,7 +201,7 @@ test.describe('Database Connection Tests', () => {
201201
expect(isConnected).toBeTruthy();
202202
});
203203

204-
test('connect PostgreSQL via UI (Manual Entry) -> verify via API', async () => {
204+
test('connect PostgreSQL via UI (Manual Entry) -> verify via API', { tag: '@requires-ai' }, async () => {
205205
test.setTimeout(120000); // Allow extra time for schema loading in CI
206206
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
207207
await browser.setPageToFullScreen();
@@ -245,7 +245,7 @@ test.describe('Database Connection Tests', () => {
245245
expect(isConnected).toBeTruthy();
246246
});
247247

248-
test('connect MySQL via UI (Manual Entry) -> verify via API', async () => {
248+
test('connect MySQL via UI (Manual Entry) -> verify via API', { tag: '@requires-ai' }, async () => {
249249
test.setTimeout(120000); // Allow extra time for schema loading in CI
250250
const homePage = await browser.createNewPage(HomePage, getBaseUrl(), 'e2e/.auth/user.json');
251251
await browser.setPageToFullScreen();
@@ -318,7 +318,7 @@ test.describe('Database Connection Tests', () => {
318318

319319
// Delete tests run serially to avoid conflicts
320320
test.describe.serial('Database Deletion Tests', () => {
321-
test('delete PostgreSQL database via UI -> verify removed via API', async () => {
321+
test('delete PostgreSQL database via UI -> verify removed via API', { tag: '@requires-ai' }, async () => {
322322
test.setTimeout(180000); // Allow extra time: schema loading + UI interaction
323323
// Use the separate postgres delete container on port 5433
324324
const postgresDeleteUrl = 'postgresql://postgres:postgres@localhost:5433/testdb_delete';
@@ -369,7 +369,7 @@ test.describe('Database Connection Tests', () => {
369369
expect(graphsList).not.toContain(graphId);
370370
});
371371

372-
test('delete MySQL database via UI -> verify removed via API', async () => {
372+
test('delete MySQL database via UI -> verify removed via API', { tag: '@requires-ai' }, async () => {
373373
test.setTimeout(180000); // Allow extra time: schema loading + UI interaction
374374
// Use the separate mysql delete container on port 3307
375375
const mysqlDeleteUrl = 'mysql://root:password@localhost:3307/testdb_delete';

0 commit comments

Comments
 (0)