feat(lab): scaffolds a basic UI test

[arein]

Breaking Changes

N/A

Changes

• feat(lab): scaffolds a basic UI test

Next: #1582

Associated Issues

closes #...

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
web3modal-gallery	✅ Ready (Inspect)	Visit Preview	Dec 18, 2023 10:22am
web3modal-laboratory	✅ Ready (Inspect)	Visit Preview	Dec 18, 2023 10:22am
web3modal-react-wagmi-ex	✅ Ready (Inspect)	Visit Preview	Dec 18, 2023 10:22am
web3modal-vue-wagmi-ex	✅ Ready (Inspect)	Visit Preview	Dec 18, 2023 10:22am

[chris13524]

1 more thing 🙏

[chris13524]

Since I don't have turbo installed, I can't run npm run playwright:test locally. We should make a npm run turbo-build -> turbo run build script and call that here instead

[arein]

I think I can remove the turbo run build from the command. Would that address your concern?

[xzilja]

You can run npm run build instead

[chris13524]

@arein the script is broken now; you can't run npm run build in the lab directory; that script does not exist. turbo run build worked because it recurses into the parent directory.

[xzilja]

You are right actually. For everything to work properly / build packages etc. Set up and commands should be ran from the monorepo root. Similarly something like npm run laboratory should be ran from there. Doing otherwise i.e. running from individual packages could have unexpected side effects probably

[arein]

ok fixed

[xzilja]

This sets everything up nicely, but does not include new approach we discussed with Jon, Sven and others for v3 testing.

Due to fast moving pace and changes, we'd need to spend substantial amount of time on re-updating tests if they are based on granular checks i.e. like you check for All Wallets.

Road forward with v3 that will keep us moving fast while providing good feedback on breaking something is through playwrights visual diff tests

More context here https://playwright.dev/docs/test-snapshots

TL;DR these are extremely cheap to set up dev time wise for existing and upcoming features while providing really good feedback on breaking changes i.e. if something is off, pixels will be off as well (missing notification. modal not open etc.). Updating these will be easy as well, i.e. if something changed and it was expected, we just update reference screenshot.

@arein Could you please switch from using "All Wallets" detection in the test, to one using screenshot diffing i.e. snap of open modal.

[xzilja]

Either needs a TODO or to be removed if unused

[xzilja]

You are right actually. For everything to work properly / build packages etc. Set up and commands should be ran from the monorepo root. Similarly something like npm run laboratory should be ran from there. Doing otherwise i.e. running from individual packages could have unexpected side effects probably

[arein]

You are right actually. For everything to work properly / build packages etc. Set up and commands should be ran from the monorepo root. Similarly something like npm run laboratory should be ran from there. Doing otherwise i.e. running from individual packages could have unexpected side effects probably

What's actually requested?
As a developer I must be able to run e2e tests from monorepo root?

This sets everything up nicely, but does not include new approach we discussed with Jon, Sven and others for v3 testing.
Due to fast moving pace and changes, we'd need to spend substantial amount of time on re-updating tests if they are based on granular checks i.e. like you check for All Wallets.
Road forward with v3 that will keep us moving fast while providing good feedback on breaking something is through playwrights visual diff tests
More context here https://playwright.dev/docs/test-snapshots
TL;DR these are extremely cheap to set up dev time wise for existing and upcoming features while providing really good feedback on breaking changes i.e. if something is off, pixels will be off as well (missing notification. modal not open etc.). Updating these will be easy as well, i.e. if something changed and it was expected, we just update reference screenshot.
@arein Could you please switch from using "All Wallets" detection in the test, to one using screenshot diffing i.e. snap of open modal.

I'm scaffolding everything here @0xasimetriq specifically to avoid comments like these. I am not actually testing anything here. The All Wallets is just a placeholder tests. I don't see how this setup cannot be used later for actual diffs

[xzilja]

Turborepo and monorepo wide commands should not be ran from here. There is global npm run build command that does exactly this in the root, can that be reused? It also sets up clean builds properly for all packages.

You will likely need to move playwright commands and test dependencies to monorepo root as well and just target tests inside laboratory folder.

[chris13524]

Due to fast moving pace and changes, we'd need to spend substantial amount of time on re-updating tests if they are based on granular checks i.e. like you check for All Wallets.

Road forward with v3 that will keep us moving fast while providing good feedback on breaking something is through playwrights visual diff tests

Taking full advantage of Playwright will involve a lot more than screenshots; every edge case should have a test case and needing to update something trivial like "All Wallets" will be the least of your concerns when changing stuff around.

[xzilja]

You are right actually. For everything to work properly / build packages etc. Set up and commands should be ran from the monorepo root. Similarly something like npm run laboratory should be ran from there. Doing otherwise i.e. running from individual packages could have unexpected side effects probably

What's actually requested? As a developer I must be able to run e2e tests from monorepo root?

This sets everything up nicely, but does not include new approach we discussed with Jon, Sven and others for v3 testing.
Due to fast moving pace and changes, we'd need to spend substantial amount of time on re-updating tests if they are based on granular checks i.e. like you check for All Wallets.
Road forward with v3 that will keep us moving fast while providing good feedback on breaking something is through playwrights visual diff tests
More context here https://playwright.dev/docs/test-snapshots
TL;DR these are extremely cheap to set up dev time wise for existing and upcoming features while providing really good feedback on breaking changes i.e. if something is off, pixels will be off as well (missing notification. modal not open etc.). Updating these will be easy as well, i.e. if something changed and it was expected, we just update reference screenshot.
@arein Could you please switch from using "All Wallets" detection in the test, to one using screenshot diffing i.e. snap of open modal.

I'm scaffolding everything here @0xasimetriq specifically to avoid comments like these. I am not actually testing anything here. The All Wallets is just a placeholder tests. I don't see how this setup cannot be used later for actual diffs

So for Q1, right now there seems to be a build:all setup in laboratory using global turborepo command. This should not be the case case, build depends on all packages, so it should be only ran and set up in root of the monorepo. I left separate suggestion about moving playwright tests to root as well.

For Q2, we should scaffold screenshot diffing flow in this pr as well, a simple single screenshot check for now will be sufficient. Idea is to have devs be able to start with this scaffold right away right, so without them needing to add this logic in. Otherwise this scaffold is likely incomplete imho.

[xzilja]

Due to fast moving pace and changes, we'd need to spend substantial amount of time on re-updating tests if they are based on granular checks i.e. like you check for All Wallets.
Road forward with v3 that will keep us moving fast while providing good feedback on breaking something is through playwrights visual diff tests

Taking full advantage of Playwright will involve a lot more than screenshots; every edge case should have a test case and needing to update something trivial like "All Wallets" will be the least of your concerns when changing stuff around.

Yes, but remember this is ui product so majority of issues will bubble up to ui, which is why screenshot diffing is great here. I.e if you set an option to disable metamask, all you have to do is screenshot that and it should not be there, if it is, then we made a regression. This applies to almost every config / action in web3modal, if something is wrong it will result in a different visual to what is expected (even text in error notifications for example)

[chris13524]

Yes, but remember this is ui product so majority of issues will bubble up to ui, which is why screenshot diffing is great here. I.e if you set an option to disable metamask, all you have to do is screenshot that and it should not be there, if it is, then we made a regression. This applies to almost every config / action in web3modal, if something is wrong it will result in a different visual to what is expected (even text in error notifications for example)

Yes, but I feel like this is the easy way out and is easier to mess up. There's 1 command that will reset all of the screenshots. So say you have a new feature that adds a new wallet to the list of wallets, and you reset all of the screenshots. You get your new wallet, but you also removed MetaMask without realizing it. Simply the screenshot checks will not detect this problem.

Now since the screenshots are committed and reviewed, hopefully someone will notice that the one called "MetaMask exists" does indeed still contain a MetaMask.

I guess my point is that it's easier to mess up an image than an explicit code removal/modifiction of the MetaMask test case.

I haven't used these snapshots before, so maybe I'll see the light :D