fix(e2e): consolidate frontend E2E to Cypress and restore CI (#967)#984
Open
DevHusnainAi wants to merge 18 commits into
Open
fix(e2e): consolidate frontend E2E to Cypress and restore CI (#967)#984DevHusnainAi wants to merge 18 commits into
DevHusnainAi wants to merge 18 commits into
Conversation
Playwright here backs Python embeddings (prompt_client), not frontend e2e. The Cypress migration replaced frontend e2e but this line was removed by mistake, breaking `make install-python`. Restore it per reviewer feedback on OWASP#774.
# Conflicts: # yarn.lock
The readiness loop broke on success but fell through to yarn test:e2e even when Flask never came up, causing confusing Cypress failures. Add a post-loop probe that exits with a clear error if the server is still unreachable. Addresses CodeRabbit review on OWASP#774.
…ings Clarify Makefile and AGENTS.md so Cypress e2e migration PRs do not drop playwright install from install-python (embeddings/scraping in prompt_client). Co-authored-by: Cursor <cursoragent@cursor.com>
Build the e2e SQLite schema from the ORM models (create_all) instead of migrate-upgrade: migrations are the Postgres path and omit columns the models added without a migration (e.g. cre.document_metadata), so a migrate-built cache is incomplete. Add a make e2e-db target and switch the workflow to install-deps + e2e-db, dropping migrate-upgrade and the unused playwright install (Cypress ships its own browser). Raise the job timeout to cover the full upstream_sync graph download. Refs OWASP#967
Add Cypress specs covering the historical Jest/Puppeteer coverage: no-results and free-text search, ASVS standard browse, CRE lookup by id and case-insensitive filter, and smartlink internal/external redirects. Each spec asserts on real seeded data so it fails on an empty database. Refs OWASP#967
Frontend e2e now runs on Cypress; drop the unused puppeteer and jest-puppeteer devDependencies and their transitive lockfile entries. Refs OWASP#967
Note how to seed the local database (make e2e-db) and run the Cypress suite (make e2e, yarn cypress:open) in README and AGENTS.md. Refs OWASP#967
DevHusnainAi
requested review from
Pa04rth,
northdpole,
paoga87 and
robvanderveer
as code owners
July 18, 2026 12:00
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughThe PR migrates frontend E2E testing from Puppeteer/Jest to Cypress, adds coverage for core routes and redirects, stabilizes Flask and database orchestration, updates CI execution, and documents local workflows. ChangesCypress E2E migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yml:
- Around line 14-15: Update the checkout step using actions/checkout@v4 to set
persist-credentials to false, preventing the GitHub token from being stored in
local Git configuration while leaving the rest of the workflow unchanged.
In `@Makefile`:
- Around line 89-101: Update the Makefile recipe containing the Flask startup
and e2e test commands so all setup, environment exports, virtualenv activation,
background PID capture, trap registration, readiness polling, and test execution
run in one shell session. Add recipe line continuations and appropriate command
separators while preserving the existing startup, failure, and cleanup behavior.
In `@README.md`:
- Around line 362-363: Update the README comment for the make e2e-db command to
state that it rebuilds the local SQLite cache on every invocation and discards
the existing cache; remove the misleading “one-time” wording while leaving the
make e2e instruction unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f9880549-6dce-4a44-b898-62a462cdd4cc
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
.github/workflows/e2e.ymlAGENTS.mdMakefileREADME.mdapplication/frontend/src/test/basic-e2e.test.tscypress.config.jscypress.jsoncypress/e2e/cre.cy.jscypress/e2e/search.cy.jscypress/e2e/smartlink.cy.jscypress/e2e/smoke.cy.jscypress/e2e/standard.cy.jspackage.json
💤 Files with no reviewable changes (2)
- application/frontend/src/test/basic-e2e.test.ts
- cypress.json
Set persist-credentials: false on the e2e checkout (the job only reads, never pushes) and correct the README: make e2e-db rebuilds and replaces the local SQLite cache on every run, it is not a one-time step. Refs OWASP#967
Author
|
Hey team! @northdpole @robvanderveer @Pa04rth @paoga87 Just a quick heads-up that this migration is ready for your review. All 5 CI/CD pipelines are passing completely green, the legacy test suite has been successfully ported over to Cypress, and I've addressed the initial CodeRabbit feedback regarding workflow credentials and documentation updates.Let me know if you need any adjustments or if this is good to land! Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #967
Consolidates the frontend E2E migration to Cypress and restores the
Test-e2eCI job, which has been disabled onmain. Builds on #774 andsupersedes #729 / #732 (both now closed). Bundles the Playwright-install
guardrail.
Python Playwright is untouched
This PR does not touch Python Playwright (embeddings/scraping in
application/prompt_client/). The diff changes zero Playwright orprompt_clientfiles, andplaywright installinmake install-pythonis unchanged. The Cypress e2e job simply doesn't invoke that target
(Cypress ships its own browser), so nothing Playwright is removed or
modified. The Python
Testworkflow (which exercises the Playwright/embedding tests) stays green.
Why CI was red, and the fix
Re-enabling
e2e.ymlas-is failed because the e2e job built the SQLitecache the wrong way:
flask db upgrade(migrations) is the Postgres path and omitscolumns the models added without a migration (e.g.
cre.document_metadata),so
upstream_syncfailed withno such column.main, which left two Alembic heads —flask db upgradeaborted with "Multiple head revisions".Fix:
upstream/main→ single Alembic head (upstream's merge migration).make e2e-dbtarget seeds the schema from the ORM models viacreate_all(the documented SQLite/CI path) then loads data withupstream_sync.e2e.ymlnow runsinstall-deps+make e2e-db+make e2e, droppingmigrate-upgradeandplaywright installfrom the frontend job.Test coverage (ported from the legacy Jest/Puppeteer suite)
smoke.cy.js/search/{term}, browse/root_cressearch.cy.jsasdf), free-text results (crypto)standard.cy.js/node/standard/ASVSheading, sections, paginationcre.cy.js558-807), case-insensitive filtersmartlink.cy.jsEach spec asserts on real seeded data, so it fails on an empty DB
rather than passing against a broken backend.
Two notes on approach
chore/playwright-install-guardrailchange (69049e36)is already present in this branch as a commit with a byte-identical
diff, so the Makefile + AGENTS.md guardrail docs land here. Cherry-picking
the original SHA would just duplicate it.
upstream_syncseeds only the CRE-linkedsubset, which doesn't include
CWE/1002. I ported the same flow usingseeded fixtures —
ASVS/V13.2.5(internal redirect) and an unmapped CWE id(external Mitre fallback) — so the test is deterministic.
Cleanup
puppeteer/jest-puppeteerdevDependencies (+ lockfile).cypress.jsonand the commentedbasic-e2e.test.ts(via fix(e2e): complete Cypress migration and stabilize make e2e for #523 #774).make e2e-db,make e2e,yarn cypress:open)in README and AGENTS.md.
Verification
All green on my fork (run):
All specs passed! 11/11(~10 min incl. fullupstream_sync)make lint,yarn build,make e2e(3× no flakes).