feat: KEEP-1351 E2E test infrastructure and happy path tests #280
Conversation
- Add unit/integration tests to PR checks workflow - Create e2e-tests.yml workflow for E2E testing - Add scheduler scripts (schedule-dispatcher, schedule-executor) - Fix docker-compose for local dev scheduler - Add Playwright happy path tests for core user journeys - Add shared test utilities (auth, workflow helpers) - Configure Playwright for deployed PR environment testing - Add workflow_run trigger to run E2E against deployed PRs - Exclude .worktrees from biome linting
- Remove skipped test files (credentials, address-book) for unimplemented routes - Move inline regex patterns to module top-level per lint rules - Fix nested ternary in playwright.config.ts - Standardize all test emails to use test+TIME@techops.services format - Fix import ordering in playwright.config.ts
- Use shared signUpAndVerify util with proper DATABASE_URL expansion - Fix openCreateOrgForm to detect already-visible form - Update input selectors from placeholder to id (#org-name, #org-slug) - Make custom slug unique with timestamp to avoid conflicts - Fix wallet overlay selector (doesn't have role=dialog) - Increase wallet creation timeout for Para API - Fix Balances heading locator - Fix refresh button enabled state wait
There was a problem hiding this comment.
Wasn't the initial idea to move the scheduler and dispatcher from this repo?
I noticed that some files we previously removed have been reintroduced in this PR. Regarding testing, I think each repository should have its own pipeline and testing suite. For the scheduler, we should mock the data we send from the API to verify that it interacts as expected.
Could you clarify the approach here and confirm whether these files should actually be in this repo? I referring specifically to scripts/schedule-dispatcher.ts and this particular one
|
|
||
| on: | ||
| push: | ||
| branches: [main, staging, production] |
There was a problem hiding this comment.
Just a typo, our production branch is called prod
There was a problem hiding this comment.
Also, do we need tests in main? That branch is used to keep the upstream in sync, but it doesn’t include most of the newly introduced functionalities.
|
|
||
| - **Test Address**: `0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045` (vitalik.eth) | ||
| - **Test Network**: `mainnet` | ||
| - **Test Cron**: `0 * * * *` (hourly), `30 8 * * 1-5` (weekday mornings) |
There was a problem hiding this comment.
This is related to a comment in the following files. Personally, I think we should rely on the infrastructure that can be run from this repository, such as webhooks and manual runs.
However, cron-based workflows should validate that the data we introduce is stored in the database as expected. The same applies to event-based workflows, since we rely on “external” services to complete the execution of the node.
Unless we set up the entire workflow by adding a pre-run step that clones both repositories into the pipeline and starts the required services, we won’t be able to fully cover these cases. That approach could be useful if we want full e2e coverage.
Otherwise, I would recommend focusing on integration tests using only what we can run within this repository.
| headers: { "Content-Type": "application/json" }, | ||
| }); | ||
|
|
||
| expect(response.status()).toBeLessThan(500); |
There was a problem hiding this comment.
Does this mean we accept any status code below 500, including 4xx responses?
| // Create wallet | ||
| await overlay | ||
| .locator('button:has-text("Create Organization Wallet")') | ||
| .click(); | ||
| await overlay.locator('button:has-text("Create Wallet")').click(); | ||
|
|
||
| // Wait for wallet creation | ||
| await expect( | ||
| page | ||
| .locator("[data-sonner-toast]") | ||
| .filter({ hasText: WALLET_CREATED_PATTERN }) | ||
| ).toBeVisible({ timeout: 30_000 }); | ||
|
|
||
| // Verify wallet address is displayed (format: 0x...xxxx) | ||
| await expect(overlay.locator("code")).toContainText(ADDRESS_PATTERN); | ||
|
|
||
| // Verify email is displayed | ||
| await expect(overlay.locator(`text=${email}`)).toBeVisible(); |
There was a problem hiding this comment.
Which Para credentials are we using to run this? Would increasing the number of users also risk exceeding the limits of our current Para plan?
joelorzet
left a comment
joelorzet
left a comment
There was a problem hiding this comment.
Based on the review feedback, there are a couple things we should align on before merging: some files that we previously moved out of this repo have been reintroduced, and we need to be careful about how we're handling credentials for the paid services included in the tests.
Also, I think moving forward each new functionality should include tests, whether unit or integration. This would really help with continuous and async work, as each developer can make clear what their changes are intended to do through tests, and it ensures we're not introducing breaking changes to existing functionality unless they're explicitly required by a ticket or request.
That said, the tests themselves are a solid foundation and provide a good entry point for expanding our coverage across the app.
2 participants
Summary
This PR establishes a comprehensive E2E testing infrastructure using Playwright, including CI integration and happy path test suites for core workflows.
New Infrastructure
.github/workflows/e2e-tests.yml) - Runs E2E tests against staging/production with proper database setuptests/e2e/playwright/utils/) - Reusable auth and workflow helpers with OTP verification from databaseTest Suites Added
auth.test.tsinvitations.test.tsorganization-wallet.test.tsschedule-trigger.test.tsworkflow.test.tshappy-paths/web3-balance.test.tshappy-paths/webhook-workflow.test.tshappy-paths/scheduled-workflow.test.tsCode Quality
test+TIME@techops.servicesformatScripts Added
scripts/schedule-dispatcher.ts- Workflow schedule dispatcherscripts/schedule-executor.ts- Workflow execution handlerTest plan
pnpm test:e2elocally against docker-compose setup