Playwright wallet connection test #1625

Velenir · 2020-11-17T08:09:26Z

Adds Wallet injection into playwright browser (window.ethereum)
Compilation for browser injects step when needed
Integration test for connecting user wallet

To see it perform and to test/debug please run

#serve the app
serve -s -l 8080 dist
#in a different terminal
yarn playwright:debug --testNamePattern=wallet

Actually please just run that so we can check it works on different machines

This reverts commit 01ee1b9.

This reverts commit cc20b5d.

gnosis-info · 2020-11-17T08:20:04Z

Travis automatic deployment:

🎩 Dapp: Testing web app
📚 Storybook: Component stories

fairlighteth · 2020-11-17T08:53:39Z

playwright/integration/connect_wallet.test.ts

+
+    // WHEN: connected a Wallet
+    // Click text=/.*Connect Wallet.*/
+    await page.click('text=/.*Connect Wallet.*/')


This doesn't look like a specific (HTML) selector but rather (regex?) looking for a specific text string. Unless I misunderstand this piece, is it better to scope down to the selectors so that their presence is validated too?

The idea is to do testing from user perspective
page.click('text=/.*Connect Wallet.*/') represents

User comes to a page

User sees text, not a selector, not even necessarily link or button

User doesn't care, text looks clickable

User clicks

That's what playwright docs recommend and what some other libraries (like react-testing-lib) advocate for

Selector presence is for unit (not even integration) testing with libs that like comparing snapshots, like Enzyme
Look up Enzyme vs react-testing-library approaches

Does that open up possible collusions/complexity around identical naming conventions for text/labels? Or do you see that as an advantage in terms of enforcing a better user experience for the user? I like the thinking here otherwise, but curious on your thoughts.

Although after reading the docs, I wonder if using an actual user-facing attribute like 'aria-label' or 'title' (etc.) would be better. It would thick both the 'selector' and 'user facing attribute/text' boxes.

Yes, it does, you are completely right. Trade offs are trade offs
That's why I use header >> text=Web3 in another click, for specificity

You can even argue, as you are saying, for better user xp. For example we have Connect Wallet both in header and in TradeOrders component (when disconnected). But they do the same, so we shouldn't care which user clicks

But the test will only click the first found item? E.g. I guess we want to be sure to test all possible instance for working properly as expected?

Yep
We want to be sure of the whole user flow
So to properly test we would need to do txs
And not rely on rinkeby random address
So setup ganache and deploy contracts locally and who knows what else

So yes,and no 😉

Makes sense.

Velenir · 2020-11-17T09:08:01Z

playwright/integration/connect_wallet.test.ts

+    // THEN: wallet connected & address available
+    // XPATH: closest ancestor with 0x* text of element with text "Copy address to clipboard"
+    const textWithAddress = await page.textContent(
+      '//*[text()="Copy address to clipboard"]//ancestor::*[starts-with(., "0x")][1]',
+    )


@biocom
This is a good example of playwright approach.
As there aren't better ways to distinguish where the address is, instead of <wallet popup class which is pseudo-random with hash 'cause generated by styled-comps> > text that looks like address
I opted for Copy address to clipboard button > closest parent with address

I think this is quite thorough, which is good. As opposed to testing for selectors.

What if there a multiple of these components doing the same thing for some reason? page.Textcontent() only returns the first found item? E.g. would be good to test the validity of any other similar/identical item on page.

Very well may be
Like here

anxolin

Do the test take so much time to you?

Also, do they fail, I get this error?

Edit, running a second time I got them working. Still slow..

cypress run it in 1sec with the video enabled. Probably can be accelerated

Is it possible is a bit flaky? Why do they’ll take so much time?

anxolin · 2020-11-17T10:23:55Z

playwright/utils/test_setup.ts

+
+beforeAll(async () => {
+  // launch chrome,headless by default
+  browser = await chromium.launch({ slowMo })


Are we hardcoding the tests to chromium?

we have to hardcode something
can of course do BROWSER=chromium|firefox|webkit and later pick it

Also can think about an optional executablePath argument, to use Chromium but use your local installed Chrome browser. Should in theory be the same I suppose (Chromium <-> Chrome) but there can be nuances in practice when testing.

Nah, can't count on Chrome extensions not screwing everything up
Because if you just use local Chrome, and have MMask installed, it will conflict with window.ethereum injected by playwright
And if you use local Chrome with --sandbox or whatever no-extension flag, then it's almost the same as Chromium

I guess my thinking is:

Even having 'expected' extensions like MM, might simulate/catch some side effects/bugs as opposed to run it in a clean env. like Chromium.

Same for running Chromium vs Chrome, like you say they are almost the same but I'd regard it as a more realistic test if you know what I mean.

Now this is interesting indeed. Can do

BROWSER=chromium EXECUTABLE_PATH=$(which google-chrome) yarn test:playwright

anxolin · 2020-11-17T13:07:59Z

package.json

@@ -31,7 +31,8 @@
    "cypress": "cypress open",
    "cypress:run": "cypress run",
    "playwright": "jest --roots playwright --no-cache",
-    "playwright:debug": "PWDEBUG=1 jest --roots playwright",
+    "playwright:rebuild": "ts-node ./playwright/utils/build_injects.ts",
+    "playwright:debug": "PWDEBUG=1 jest --roots playwright --runInBand",


Good call creating this script

Velenir · 2020-11-17T13:14:11Z

Is it possible is a bit flaky? Why do they’ll take so much time?

This is strange. How did you run? what command?
@biocom , @W3stside can you also try
First yarn test:playwright for the whole test
After

#serve the app
serve -s -l 8080 dist
#in a different terminal
yarn playwright:debug --testNamePattern=wallet

anxolin · 2020-11-17T13:18:05Z

With test:playwrite is taking less time 8s and succeeding

The debug worked too now

Velenir · 2020-11-18T09:03:25Z

Added BROWSER and EXECUTION_PATH env vars.
Would be cool if you tried

serve -s -l 8080 dist

BROWSER=chromium EXECUTABLE_PATH=$(which google-chrome) yarn playwright #or brave browser
BROWSER=firefox yarn playwright
BROWSER=webkit yarn playwright

webkit doesn't work for me, though, but it worked for @W3stside

anxolin

Very good job.
The injection was a bit hacky, I mean. programmatically using webpack to compile the wallet, ...

I haven't gone over all the doc, so not sure if there's simpler way. I would think so, but just glad u found a solution.

anxolin · 2020-11-19T11:01:49Z

playwright/integration/connect_wallet.test.ts

+
+    // WHEN: connected a Wallet
+    // Click text=/.*Connect Wallet.*/
+    await page.click('text=/.*Connect Wallet.*/')


Why do you use a "pattern", is not needed right? (since text selector already searches for partial text matching)

Also, I think it's more readable the short-form

I've try it out here, feel free to merge if u agree #1631

This pattern was made by playwright itself during playwright:record, I just didn't think twice to change it

anxolin · 2020-11-19T15:01:27Z

playwright/utils/test_setup.ts

+jest.setTimeout(30000)
+if (isDebug) {
+  // slow down playright actions
+  slowMo = 2000


Maybe add a const on top, and explain that artificially delay the test to visually see what is going on

Simplify text selectors

Velenir added 23 commits November 17, 2020 11:02

remove extras

9e354ec

abstract away test setup

5fc0fc9

use test_setup

42270ad

add truffle/hdwallet-provider

5181c9f

setup connect_wallet test

b550428

add provider_setup

7817ad0

better xpath query

5fc29e6

conditionally compile provider

171760f

env var names in line with PWDEBUG

fc4226f

Revert "better xpath query"

64ba531

This reverts commit 01ee1b9.

better xpath v2

892eaa0

abstract away compiler function

7658240

move injects to a folder

8480bec

add ts-node

62df26c

add ts-node TS config

312609f

add script for explicit playwright injects build

d404936

setup tests as functions

cbd20f6

Revert "setup tests as functions"

2022842

This reverts commit cc20b5d.

--runInBand when debugging playwright

329ef47

don't commit playwright/build

c2dec07

cleanup

6f5b576

add comments

88592ca

cleanup

603f285

Velenir requested review from anxolin and W3stside November 17, 2020 08:09

fairlighteth reviewed Nov 17, 2020

View reviewed changes

Velenir commented Nov 17, 2020

View reviewed changes

anxolin reviewed Nov 17, 2020

View reviewed changes

fairlighteth approved these changes Nov 17, 2020

View reviewed changes

Velenir added 3 commits November 18, 2020 11:42

add BROWSER flag

9d31627

add EXECUTABLE_PATH flag

296be96

longer jest timeout

2fa549b

W3stside approved these changes Nov 18, 2020

View reviewed changes

remove --no-cache

444b09d

anxolin mentioned this pull request Nov 19, 2020

Simplify text selectors #1631

Merged

Change selectors

1afc557

anxolin approved these changes Nov 19, 2020

View reviewed changes

anxolin and others added 2 commits November 19, 2020 17:39

Merge pull request #1631 from gnosis/simplify-text-selectors

2724138

Simplify text selectors

add VISUAL_ACTION_DELAY const

2743614

Velenir merged commit e2e9d41 into playwright_integration Nov 20, 2020

Velenir deleted the playwright_wallet_test branch November 20, 2020 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Playwright wallet connection test #1625

Playwright wallet connection test #1625

Velenir commented Nov 17, 2020

gnosis-info commented Nov 17, 2020

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020

Velenir Nov 17, 2020

fairlighteth Nov 17, 2020

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020 •

edited

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020 •

edited

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020

anxolin left a comment •

edited

anxolin Nov 17, 2020

Velenir Nov 17, 2020

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020 •

edited

fairlighteth Nov 17, 2020

Velenir Nov 17, 2020 •

edited

anxolin Nov 17, 2020

Velenir commented Nov 17, 2020

anxolin commented Nov 17, 2020 •

edited

Velenir commented Nov 18, 2020 •

edited

anxolin left a comment

anxolin Nov 19, 2020

anxolin Nov 19, 2020

Velenir Nov 20, 2020

anxolin Nov 19, 2020

Playwright wallet connection test #1625

Playwright wallet connection test #1625

Conversation

Velenir commented Nov 17, 2020

gnosis-info commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Velenir Nov 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Velenir Nov 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anxolin left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Velenir Nov 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Velenir Nov 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Velenir commented Nov 17, 2020

anxolin commented Nov 17, 2020 • edited

Velenir commented Nov 18, 2020 • edited

anxolin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Velenir Nov 17, 2020 •

edited

Velenir Nov 17, 2020 •

edited

anxolin left a comment •

edited

Velenir Nov 17, 2020 •

edited

Velenir Nov 17, 2020 •

edited

anxolin commented Nov 17, 2020 •

edited

Velenir commented Nov 18, 2020 •

edited