test: add browser interaction tests, formatter/pipeline coverage, and CI browser support#39
Closed
michnicki wants to merge 10 commits into
Closed
test: add browser interaction tests, formatter/pipeline coverage, and CI browser support#39michnicki wants to merge 10 commits into
michnicki wants to merge 10 commits into
Conversation
Release v0.9.2: UI Redesign, Custom LLM Providers, Resumable Reviews & Setup Automation
Release v0.9.4: job cancellation, telemetry opt-out, concurrency limits & motion UI
Splits vitest into node and browser projects so UI interaction tests can run against real Chromium via @vitest/browser-playwright instead of jsdom. Migrates the existing dashboard spec into test/browser/ and adds a test:browser script that runs independently of the Postgres/env setup the node project requires.
Covers real click/type/select interactions the previous jsdom-only setup couldn't exercise: search debouncing and page reset, the Radix status dropdown, the filtered empty state, pagination controls, and theme persistence via real matchMedia/localStorage.
Exercises the largest untested page surfaces: provider API-key editing and save-failure handling in Settings, repo enable/pause toggling and GitHub sync in Repos, and finding expansion plus the re-run/retry button in the job detail view.
The PR comment/summary formatter had zero test coverage despite being on the critical path for every posted review. Covers verdict mapping, severity icons, legacy tag stripping, title/body dedupe, and the review overview markdown.
Every existing review-flow test mocks @server/services/github and @server/services/model entirely, so the real GitHubClient (check runs, review posting, the 422 retry-without-comments fallback, label lifecycle) and the real ModelService were never exercised together. Adds installGitHubFetchMock, which stubs global fetch at the api.github.com boundary instead of mocking the service modules, plus test helpers to seed a cached installation token (bypassing JWT/crypto entirely) and a default model strategy. The new spec drives a signed webhook through the full prepare/review/finalize pipeline with zero vi.mock calls, asserting on the actual GitHub API requests: check-run lifecycle, inline comment positions computed from a real diff, review body formatting, and label create/add. Also extracts the webhook signing helper out of webhook-handling.spec into test/helpers so both specs share it.
Adds a cached Playwright Chromium install step and a Browser Tests step to the existing verify job so the new UI interaction suite runs on every PR. Also ignores vitest browser-mode screenshot/attachment directories generated locally on failure.
…npm audit findings @cloudflare/vitest-pool-workers was declared but never wired into vitest.config.ts or referenced anywhere in the codebase. Its pinned transitive deps (esbuild, miniflare, wrangler, undici, ws) accounted for 7 of the 9 reported vulnerabilities (3 critical, 5 high), so it's removed outright rather than force-upgraded to a breaking major. vitest/@vitest/browser/@vitest/browser-playwright were resolving to 4.1.7, which has a critical Vitest Browser Mode RCE advisory (GHSA-g8mr-85jm-7xhm). A clean reinstall (needed since these packages peer-pin each other to an exact matching version) resolved all three to the patched 4.1.10 within the existing ^4.1.4 range. npm audit now reports 0 vulnerabilities. Full node (119) and browser (19) suites verified green against the updated dependencies.
Rebasing onto the new upstream main surfaced real breakage beyond
merge conflicts: the review pipeline moved from queue re-enqueuing to
Cloudflare Workflows, DLQ was removed, ModelConfig dropped its
per-model rpm/tpm/rpd fields, job-detail gained separate rerun/stop/
delete actions, and the Select component switched from a Radix menu
to a custom listbox.
- pr-review-pipeline.spec.ts: runReviewJob no longer re-enqueues via
REVIEW_QUEUE, it returns {action:'next_phase', phase, delaySeconds}
for the caller to chase. Replaces the queue-drain loop with the same
runAndDrain pattern review-flow.spec.ts uses, and defensively clears
stray 'running' jobs before each test since global (unscoped)
concurrency admission control otherwise throttles fresh jobs left
over by other specs in the shared test DB.
- github-fetch-mock.ts: stop passing non-GitHub hosts through to the
real network (core/telemetry.ts fires a real POST to codra.run on
every finalize) and add the GET .../reviews route
findBotReviewForCommit uses.
- helpers.ts: drop rpm/tpm/rpd from seedDefaultModelStrategy's
updateModelConfig call.
- jobs.spec.tsx: remove the deleted getDlqMessages mock, query Select
options by role="option" instead of the old Radix "menuitem", and
match the current pagination/empty-state copy.
- settings.spec.tsx: drop rpm/tpm/rpd from the model fixture, mock the
new getReviewSettings call the page now makes on load, and assert
against the provider's model-count text instead of a since-removed
flat model list.
- job-detail.spec.tsx: switch from the removed retryJob to rerunJob.
Full node (188) and browser (19) suites verified green, twice in a
row to confirm the admission-control fix isn't order-dependent.
![codra-app-personal[bot]](https://avatars.githubusercontent.com/in/3347243?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Owner
|
Not sure why this PR was closed @michnicki Really appreciate the contribution, it was in the right direction for #10 ## How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
- [ ] Unit Tests
- [ ] Integration Tests
- [ ] Manual Dashboard Verification
- [ ] Manual GitHub Webhook Verification
## Checklist:
- [ ] I have starred Codra on GitHub
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] My changes generate no new warnings
- [ ] New and existing unit tests pass locally with my changes
- [ ] I have signed the [CLA](https://codra.run/cla) |
Author
|
Sorry, I don't want to sign the CLA. |
Owner
Understood, that’s your call to make. Appreciate you being upfront about it. |
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
2 participants
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.
Summary
Test plan
npm testpasses locally