-
Notifications
You must be signed in to change notification settings - Fork 0
chore: unify qa automation #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Install Backend Dependencies | ||
| run: cd backend && bun install | ||
|
|
||
| # TODO: Install frontend dependencies | ||
| # - name: Install Frontend Dependencies | ||
| # run: cd frontend && bun install | ||
| - name: Install Frontend Dependencies | ||
| run: cd frontend && bun install | ||
|
|
||
| # TODO: Build frontend | ||
| # - name: Build Frontend | ||
| # run: cd .cursor && task frontend:build | ||
| - name: Start Backend | ||
| run: | | ||
| cd backend | ||
| encore run & | ||
| echo "Waiting for backend to be ready..." | ||
| timeout 60 bash -c 'until curl -sf http://localhost:4000/health > /dev/null; do sleep 2; done' | ||
|
|
||
| # TODO: Start frontend | ||
| # - name: Start Frontend | ||
| # run: cd .cursor && task frontend:dev & | ||
| # sleep 5 | ||
|
|
||
| # TODO: Run smoke tests | ||
| # - name: Run Frontend Smoke Tests | ||
| # run: bun run qa:smoke:frontend | ||
|
|
||
| # ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
| # Job 4: Type Checking | ||
| # ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
| # Additional quality check | ||
| # Command: bun run typecheck | ||
|
|
||
| typecheck: | ||
| name: TypeScript Type Checking | ||
| runs-on: ubuntu-latest | ||
| needs: founder-rules | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| - name: Start Frontend | ||
| run: | | ||
| cd frontend | ||
| bun run dev & | ||
| echo "Waiting for frontend to be ready..." | ||
| timeout 60 bash -c 'until curl -sf http://localhost:5173 > /dev/null; do sleep 2; done' | ||
|
|
||
| # TODO: Setup and run typecheck | ||
| # - name: Run TypeScript Type Checking | ||
| # run: bun run typecheck | ||
| - name: Run Complete QA Suite | ||
| run: cd .cursor && task qa:all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Install Playwright browsers in CI job
The new QA workflow runs task qa:all, which ends up executing playwright test for the frontend. The job installs Node dependencies but never downloads the Playwright browser binaries, so the step will consistently fail on a fresh GitHub runner with an error like “Please install browsers with npx playwright install”. Add a browser installation step (e.g. npx playwright install --with-deps) before running the QA suite.
Useful? React with 👍 / 👎.
- Changed from ENCORE_AUTH_TOKEN to ENCORE_AUTH_KEY (app-specific) - Updated docs to clarify auth key vs personal token difference - Auth key must be created in Encore dashboard, not via 'encore auth token' - Fixes 500 error caused by using wrong credential type
- badge.svelte imports tailwind-variants but it wasn't in package.json - Passed locally due to persistent node_modules - Failed in CI with fresh install - Fixes typecheck error in CI workflow
|
|
||
| // Import after mocking to get the mocked version | ||
| const childProcess = await import("node:child_process"); | ||
| const { promisify } = await import("node:util"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } as never); | ||
| } | ||
| return {} as never; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incorrect exec Mock Callback Signature
The exec callback mock has incorrect signature. Node.js exec callbacks receive three separate arguments (error, stdout, stderr), but the mock passes an object { stdout, stderr } as the second argument. This breaks promisify(exec) which expects the standard three-argument callback pattern.
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
…ooks Problem: Running qa:fix inside qa:all during git hooks causes auto-fixes without staging changes, leading to commits without the fixes. Solution: - qa:all now validation-only (no code modification) - Created qa:all:fix for manual workflow (fix → validate) - Updated hooks and CI docs to clarify validation-only behavior Rationale: - Git hooks should validate, not modify uncommitted code - CI should validate, not modify code (anti-pattern) - Manual workflow: run qa:all:fix, review changes, stage, commit Files: - .cursor/commands/qa/Taskfile.yml - Split validation from fixing - .github/workflows/ci.yml - Updated docs - .husky/pre-commit - Clarified validation-only - .husky/pre-push - Clarified validation-only
Note
Activates a CI workflow running the unified validation QA suite, switches Husky hooks to validation-only, adds a fix→validate task, improves device-check tests with adb mocks, and adds a frontend dependency.
.github/workflows/ci.ymlto runcd .cursor && task qa:allwith environment setup (Encore auth, bun/node, deps, start backend/frontend, Playwright install).qa:allbecomes validation-only (rules + smoke + lint + typecheck + unit + e2e) and addqa:all:fix(auto-fix then validate)..husky/pre-commitand.husky/pre-pushto run validation-onlytask qa:all(remove auto-fix); keep merge-commit check on pre-push.backend/agent/nodes/setup/EnsureDevice/device-check.test.tsto mockchild_process.execand add precise tests for single/multiple devices, no devices, missing device, and adb failures.tailwind-variantstofrontend/package.json(lockfile updated).Written by Cursor Bugbot for commit 45e386a. This will update automatically on new commits. Configure here.