Skip to content

Add playwright setup with demo test#19

Closed
agualis wants to merge 11 commits intoaave-dao:mainfrom
agualis:test/e2e-tests
Closed

Add playwright setup with demo test#19
agualis wants to merge 11 commits intoaave-dao:mainfrom
agualis:test/e2e-tests

Conversation

@agualis
Copy link
Copy Markdown

@agualis agualis commented Jun 2, 2025

Adds initial playwright setup and demo covering several happy paths

  • Sets up an alternative e2e wagmi config to use an anvil fork (for now only for base sepolia) in e2e tests
  • Adds Github Actions job to run e2e tests
  • Adds test demo to show how to test a staking flow: the default anvil account has 10000 ETH by default, so we can always use ETH for stake and claim. More sophisticated scenarios could be tested with some more setup.

Test execution in UI mode:

playwright-demo.mp4

Blockers

  • getAllAggregatedData function is quite complex and it takes a long time to run in the context of an anvil fork. For now, I'm mocking the response of useAllStkTokens when running it in CI but that's far from ideal so I will think about a better way. Added anvil warm up script to load getAllAggregatedData call before running e2e tests so that they are using anvil's cache.

Open questions

  • eslint and typecheck were failing in main when I forked so I had to temporary disable it in next.config.ts. Wouldn't it be interesting to add a job to disable merging to main when those are broken?
  • I used build --turbopack for building the app in e2e in CI mode. This mode is not yet ready for production but provides a faster feedback loop. So there's a tradeoff between detecting production build errors using webpack build VS speed.
  • ssr option in wagmi is set to true in e2e tests to avoid hydration issues. Why do you have it disabled?
  • Some test selectors are quite flaky (i.e. await page.getByRole("combobox").click();). Improving accesibility would be good for both real users and e2e tests.
  • react-scan is activated by default. That affects the DX of the playwright UI mode so I disabled it in that context. Now I see Tanstack devtools button instead so it looks like there is a collision there.
  • Tests are only run for chromium. It would be straighforward to run them for other browsers or viewport setups.
  • In order to wallets RainbowKit (RK) option, I had to manually define groupName: Popular with RK defaults. I guess you would prefer to define your own ones, which is easier now.

Nice to haves

  • Current demo test is deterministic cause the default anvil account (const defaultAnvilAccount: Address = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266";) will always have enough ETH to stake many times in subsequent tests but adding a resetFork function to reset state between test executions would be a good practice.

@agualis agualis marked this pull request as draft June 2, 2025 08:37
@agualis agualis marked this pull request as ready for review June 2, 2025 16:13

export const isE2eTestEnabled = process.env.NEXT_PUBLIC_E2E_TEST_ENABLED === "true";
if (isE2eTestEnabled) console.warn("E2E tests are enabled. This should only be used in a testing environment.");
export const baseSepoliaE2eForkUrl = "http://127.0.0.1:8545";
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be extracted to env vars and extended to e2e test with multiple chains.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general rule, we prefer to use environment variables for scenarios like this. It makes things much smoother for both local development and when deploying to different environments on Vercel

Comment thread e2e/tests/demo.spec.ts Outdated
Comment thread next.config.ts
const nextConfig: NextConfig = {
// FIXME: Delete this setup where TS errors in main are fixed
eslint: {
ignoreDuringBuilds: true,
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main failing currently with:

Screenshot 2025-06-02 at 18 20 46

Comment thread vitest.config.mts
environment: "jsdom",
globals: true,
setupFiles: ["./src/test/setup.ts"],
exclude: ["**/node_modules/**", "**/dist/**", "**/e2e/**"],
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoids running e2e tests from vitest. Using another filename convention to distinguish unit vs e2e would also be a good improvement.

agualis and others added 7 commits June 3, 2025 10:56
* Add env var to test scripts

* Improve e2e readme

* Add NEXT_PUBLIC_E2E_TEST_ENABLED env var to GHA yaml

* Add default RPC url for GHA yaml

* Change default e2e timeout

* Mock allStkTokens with disabled query

* Comment steps in tests
Alberto Gualis and others added 3 commits June 3, 2025 11:00
* add anvil warmup

* rename GHA job

* change GHA push on pull_request

* add disable-block-gas-limit

* undo useAllStkTokens changes

* change Claim All button label

* change Cooldown label

* wait for claimable rewards
@isArlekin
Copy link
Copy Markdown
Collaborator

Hi @agualis!

Thanks a lot for contributing and opening this PR! We appreciate you taking the time to add e2e tests with Playwright to the project.
I'll be sure to review it and provide my comments shortly.

@isArlekin
Copy link
Copy Markdown
Collaborator

Regarding the Testing Environment

Before you put too much more work into this PR, I wanted to share some thoughts on the testing environment. We've actually been planning to implement our e2e tests using a Tenderly Fork rather than Anvil.

There are a few reasons for this, one of which you've already touched upon: running complex functions on Anvil can be quite time-consuming and can make test execution slow/unpredictable. With a Tenderly Fork, we don't face this issue. Our plan involves creating a separate fork via the Tenderly API for each test run, which allows us to manage wallet balances, rewind time for reward accrual, and handle cooldown execution effectively.

Comment thread src/hooks/useAllStkTokens.ts Outdated
Comment thread src/hooks/useAllStkTokens.ts Outdated
@agualis
Copy link
Copy Markdown
Author

agualis commented Jun 11, 2025

Closing in favor of #29 as discussed above.
New PRs will be created to add more tests and improve tederly setup.

@agualis agualis closed this Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants