-
Notifications
You must be signed in to change notification settings - Fork 154
chore: Deprecate button clicks for tests #195
Conversation
@@ -6,22 +6,9 @@ | |||
<meta http-equiv="X-UA-Compatible" content="IE=edge"> |
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.
I kept this empty page voluntarily, so as not to load any remote page, and have to wait for any network requests. I really think it's cleaner than loading any website. Let me know if you have a strong opinion regarding this.
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.
Overall looks good
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.
Over all looks good, just one question below
test/basic.spec.ts
Outdated
}); | ||
|
||
it("should not add network", async () => { | ||
await clickElement(testPage, ".add-network-button"); | ||
const addNetworkPromise = testPage.evaluate(addNetwork); | ||
await metamask.page.waitForTimeout(500); |
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.
This waitForTimeout
on metamask page seems like a red flag. Is there any possibility to avoid it?
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.
You're totally right, I'm not sure why I ended up adding them.. I imagine to avoid some flakiness I experienced.
There is in my opinion no way to avoid these because we're triggering an action on a tab, and hope for the extension in the other tab to catch it before our next function kicks in. There's nothing we can wait for unfortunately beside let some time pass.
I removed them, let's see how things go. The AddToken
tests don't have any wait and are flacky.
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.
then using a pause
from utils
is a better solution than pausing the MetaMask page.
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.
ah I see what you mean, sounds good, I'll use this in the future if there's a need.
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.
LGTM
* chore: update CI for unstable branch * fix!: casing of MetaMask (#132) Fix casing of MetaMask Co-authored-by: Marin Petrunic <[email protected]> * fix: update ganache, fix test depending on goerli (#144) * fix: selector issues, useless timeouts, reorganise tests (#145) * fix: update ganache, fix test depending on goerli * fix: bugs in selectors, less timeout, reorganise structure * update yarn * chore: improve root hooks * try to increase timeout * better ci * add screenshot uploading to ci * fix screenshot name * run screenshot on failed test * fix lint * change viewport * fix tests * fix#123124 * fix tests * whyyyyyyyyyyyyyyyyyyyyyyyyy * please * fix testing dapp * dunno anymore * fix lint * fix typo in selector * update dapp loading * refactor script fetching for `index.html` * fix dam sing test Co-authored-by: Bernard <[email protected]> * fix: import token flaky (#149) fix: import-token flakyness * feat!: update recommended metamask version (#151) * feat!: update recommended metamask version * implement screenshot helper * implement hide portfolio popup action * implement close what's new modal action * remove obsolete test * fix dappeteer methods broken on update * chore: fix lint * update metamask version to `10.20.0` * fix addToken.ts * address issues * fix lint * fix tests * fix #4 * fix non visible buttons Co-authored-by: Bernard Stojanović <[email protected]> * chore: change eslint config to chainsafe shared (#152) * chore: change eslint to chainsafe config * address PR comments * address PR comments * fix tests * feat: add support for installing metamask flask (#153) feat: add support for installing metamask flaask * feat: ability to install snap (#154) * feat: add support for installing metamask flaask * feat: ability to install snaps * fi lint * fix starting a snap * fix snap install Co-authored-by: Bernard <[email protected]> * fix: snap install faster, run all tests (#163) * fix: run all tests, faster check for installed snap * fix condition bug * feat: Add invokeSnap method; update installSnap method parameter; (#159) * Add invokeSnap method; update installSnap method parameter; * adjust invokeSnap method after review * Fix params type in invokeSnap method * update invokeSnap Return type * update invokeSnap with generic params * update test expectation * fix lint * feat: add ability to accept dialogs (#138) (#164) * feat:add ability to accept dialogs (#138) * fix lint * move installSnap, invokeSnap to dappeteer api; add bringToFront call to dialog methods * pair improvements * fix lint * add methods snap * exclude flask tests from global * exclude flask tests from global Co-authored-by: Bernard <[email protected]> * feat!: add playwright support (#167) * feat!: add playwright support * fix: lint * fix: ci matrix * don't run snap tests on non flask * address PR comments * feat: added notification snap to methods-snap #137 (#166) * pair improvements * fix lint * feat: add notify Snap method * Added getAllNotifications method * go back after getting all notifications * remove timeout * fix post merge errors Co-authored-by: Bernard <[email protected]> Co-authored-by: Marin Petrunic <[email protected]> * feat: method to bootstrap snap env (#180) * feat: install snap from local files * fix: installing and invokeing snaps * disable addToken test * disable fail fast * replace binance smart chain * chore: node engine requirements (#184) set engines compatibility for 16 and above * chore: remove metamask dir (#185) * fix: remove page param from install snap (#188) * feat!: replace outdated methods (#189) * feat!: replaced addTokenMethod * feat: remove add network method * chore: Ci enhancement (#193) * cache yarn and have lint and build as separate steps * add back yarn.lock * add jobs * fix name * feat: allow signing typed data (#191) * add test to sign typed data * add a check to see if the sign button is actually disabled * clean up * fix and clean * remove flackyness * lint * fix playwright * add hidden option for pupeteer types waitForSelector * remove reload between tests * fix: fix prompt clicking flakiness, fix multiple snap key permissions (#194) * add retries when confirming prompts with lower timeout * better snap error handling, ability to accept multiple key permissions * bail on first fail * add context to screenshots * remove various random timeouts, add wait for overlay, more retrys * fix lint * increase timeout for loading network and token details * address PR comments * increase timeout * add retry to profile dropdown * feat: snap notifications 137 (#187) * chore: node engine requirements (#184) (#186) set engines compatibility for 16 and above Co-authored-by: Bernard Stojanović <[email protected]> * pair improvements * fix lint * feat: add notify Snap method * go back after getting all notifications * remove timeout * wip * Add notification observe method * update test * remove unused code * wip notifiction observer * wip observer based notifications * wip observer based notifications * remove unused dependency * cleanup * fixes after merge * revert method names * clean waitForNotification method * emitter solution - FP * emitter solution - ClassBased * Cleanup class based emitter * fix lint * fixes after merge * remove p-event library * remove eslint comment * update test configuration * return NotificationList from waitForNotification * add getNotificationEmitter method * remove back button Co-authored-by: Marin Petrunić <[email protected]> Co-authored-by: Bernard Stojanović <[email protected]> Co-authored-by: Marin Petrunic <[email protected]> * chore: Deprecate button clicks for tests (#195) * sharedConst and signature check * without sharedConst :) * request accounts * request accounts * long typed data * short typed data * add token reject/accept * add network accept/reject * actually verify the lock/unlock * contract interactions * address comment * web3-utils 1.3.4 * addToken and addNetwork use boolean * remove useless wait * chore: Update documentation and Readme (#202) * docs * change to metaMask accross the board and use bootstrap in our test * MetaMMMMMMask * snaps methods in API.md * snaps usage in Readme.md * Update README.md Co-authored-by: Marin Petrunić <[email protected]> * address comment * add forgotten menu item for initSnapEnv * removing jsdoc Co-authored-by: Marin Petrunić <[email protected]> * chore: Remove local server for dapp (#203) * docs * change to metaMask accross the board and use bootstrap in our test * MetaMMMMMMask * snaps methods in API.md * snaps usage in Readme.md * init * lint * add dev deps for ganache/console.log to make build pass * Update README.md Co-authored-by: Marin Petrunić <[email protected]> * address comment * add forgotten menu item for initSnapEnv * removing jsdoc Co-authored-by: Marin Petrunić <[email protected]> * feat: Simplify `installSnap` and `initSnapEnv` apis (#206) * simplify api * remove stray comments * fix undefined opts * double question mark back :) * Update src/snap/utils.ts * 2s for my use case * test perf * elegant race with bad name * chore: update web3 (#217) chore: upgrade web3.js * chore: update metamask version (#209) * update recommended version * chore: fix timeout for accept dialog (#218) increase timeout for accept and reject dialog * fix: increase viewport size (#219) * Set viewport to most popular desktop resolution; fix for #208 Co-authored-by: Bernard Stojanović <[email protected]> * fix: viewport typo (#220) fix viewport typo Co-authored-by: Maarten Zuidhoorn <[email protected]> Co-authored-by: Bernard <[email protected]> Co-authored-by: Anton Lykhoyda <[email protected]> Co-authored-by: Thibaut Sardan <[email protected]>
* sharedConst and signature check * without sharedConst :) * request accounts * request accounts * long typed data * short typed data * add token reject/accept * add network accept/reject * actually verify the lock/unlock * contract interactions * address comment * web3-utils 1.3.4 * addToken and addNetwork use boolean * remove useless wait
* sharedConst and signature check * without sharedConst :) * request accounts * request accounts * long typed data * short typed data * add token reject/accept * add network accept/reject * actually verify the lock/unlock * contract interactions * address comment * web3-utils 1.3.4 * addToken and addNetwork use boolean * remove useless wait
* chore: update CI for unstable branch * fix!: casing of MetaMask (#132) Fix casing of MetaMask Co-authored-by: Marin Petrunic <[email protected]> * fix: update ganache, fix test depending on goerli (#144) * fix: selector issues, useless timeouts, reorganise tests (#145) * fix: update ganache, fix test depending on goerli * fix: bugs in selectors, less timeout, reorganise structure * update yarn * chore: improve root hooks * try to increase timeout * better ci * add screenshot uploading to ci * fix screenshot name * run screenshot on failed test * fix lint * change viewport * fix tests * fix#123124 * fix tests * whyyyyyyyyyyyyyyyyyyyyyyyyy * please * fix testing dapp * dunno anymore * fix lint * fix typo in selector * update dapp loading * refactor script fetching for `index.html` * fix dam sing test Co-authored-by: Bernard <[email protected]> * fix: import token flaky (#149) fix: import-token flakyness * feat!: update recommended metamask version (#151) * feat!: update recommended metamask version * implement screenshot helper * implement hide portfolio popup action * implement close what's new modal action * remove obsolete test * fix dappeteer methods broken on update * chore: fix lint * update metamask version to `10.20.0` * fix addToken.ts * address issues * fix lint * fix tests * fix #4 * fix non visible buttons Co-authored-by: Bernard Stojanović <[email protected]> * chore: change eslint config to chainsafe shared (#152) * chore: change eslint to chainsafe config * address PR comments * address PR comments * fix tests * feat: add support for installing metamask flask (#153) feat: add support for installing metamask flaask * feat: ability to install snap (#154) * feat: add support for installing metamask flaask * feat: ability to install snaps * fi lint * fix starting a snap * fix snap install Co-authored-by: Bernard <[email protected]> * fix: snap install faster, run all tests (#163) * fix: run all tests, faster check for installed snap * fix condition bug * feat: Add invokeSnap method; update installSnap method parameter; (#159) * Add invokeSnap method; update installSnap method parameter; * adjust invokeSnap method after review * Fix params type in invokeSnap method * update invokeSnap Return type * update invokeSnap with generic params * update test expectation * fix lint * feat: add ability to accept dialogs (#138) (#164) * feat:add ability to accept dialogs (#138) * fix lint * move installSnap, invokeSnap to dappeteer api; add bringToFront call to dialog methods * pair improvements * fix lint * add methods snap * exclude flask tests from global * exclude flask tests from global Co-authored-by: Bernard <[email protected]> * feat!: add playwright support (#167) * feat!: add playwright support * fix: lint * fix: ci matrix * don't run snap tests on non flask * address PR comments * feat: added notification snap to methods-snap #137 (#166) * pair improvements * fix lint * feat: add notify Snap method * Added getAllNotifications method * go back after getting all notifications * remove timeout * fix post merge errors Co-authored-by: Bernard <[email protected]> Co-authored-by: Marin Petrunic <[email protected]> * feat: method to bootstrap snap env (#180) * feat: install snap from local files * fix: installing and invokeing snaps * disable addToken test * disable fail fast * replace binance smart chain * chore: node engine requirements (#184) set engines compatibility for 16 and above * chore: remove metamask dir (#185) * fix: remove page param from install snap (#188) * feat!: replace outdated methods (#189) * feat!: replaced addTokenMethod * feat: remove add network method * chore: Ci enhancement (#193) * cache yarn and have lint and build as separate steps * add back yarn.lock * add jobs * fix name * feat: allow signing typed data (#191) * add test to sign typed data * add a check to see if the sign button is actually disabled * clean up * fix and clean * remove flackyness * lint * fix playwright * add hidden option for pupeteer types waitForSelector * remove reload between tests * fix: fix prompt clicking flakiness, fix multiple snap key permissions (#194) * add retries when confirming prompts with lower timeout * better snap error handling, ability to accept multiple key permissions * bail on first fail * add context to screenshots * remove various random timeouts, add wait for overlay, more retrys * fix lint * increase timeout for loading network and token details * address PR comments * increase timeout * add retry to profile dropdown * feat: snap notifications 137 (#187) * chore: node engine requirements (#184) (#186) set engines compatibility for 16 and above Co-authored-by: Bernard Stojanović <[email protected]> * pair improvements * fix lint * feat: add notify Snap method * go back after getting all notifications * remove timeout * wip * Add notification observe method * update test * remove unused code * wip notifiction observer * wip observer based notifications * wip observer based notifications * remove unused dependency * cleanup * fixes after merge * revert method names * clean waitForNotification method * emitter solution - FP * emitter solution - ClassBased * Cleanup class based emitter * fix lint * fixes after merge * remove p-event library * remove eslint comment * update test configuration * return NotificationList from waitForNotification * add getNotificationEmitter method * remove back button Co-authored-by: Marin Petrunić <[email protected]> Co-authored-by: Bernard Stojanović <[email protected]> Co-authored-by: Marin Petrunic <[email protected]> * chore: Deprecate button clicks for tests (#195) * sharedConst and signature check * without sharedConst :) * request accounts * request accounts * long typed data * short typed data * add token reject/accept * add network accept/reject * actually verify the lock/unlock * contract interactions * address comment * web3-utils 1.3.4 * addToken and addNetwork use boolean * remove useless wait * chore: Update documentation and Readme (#202) * docs * change to metaMask accross the board and use bootstrap in our test * MetaMMMMMMask * snaps methods in API.md * snaps usage in Readme.md * Update README.md Co-authored-by: Marin Petrunić <[email protected]> * address comment * add forgotten menu item for initSnapEnv * removing jsdoc Co-authored-by: Marin Petrunić <[email protected]> * chore: Remove local server for dapp (#203) * docs * change to metaMask accross the board and use bootstrap in our test * MetaMMMMMMask * snaps methods in API.md * snaps usage in Readme.md * init * lint * add dev deps for ganache/console.log to make build pass * Update README.md Co-authored-by: Marin Petrunić <[email protected]> * address comment * add forgotten menu item for initSnapEnv * removing jsdoc Co-authored-by: Marin Petrunić <[email protected]> * move temporary user data dir in upper scope * implement exporting state and running from a state * implement userDataDir for puppeteer * implement tests * fix setupBootstrappedMetaMask for flask * fix flask * implement addKeyToMetaMaskManifest * small fixes * improve userData testing * fix jest config * implement default user profile * small quality of life improvements * improve documentation * fix headless issues whit handling files * improve ci * fix yml * fix ci * improve ci * fix export * address comments Co-authored-by: Marin Petrunic <[email protected]> Co-authored-by: Maarten Zuidhoorn <[email protected]> Co-authored-by: Marin Petrunić <[email protected]> Co-authored-by: Anton Lykhoyda <[email protected]> Co-authored-by: Thibaut Sardan <[email protected]>
Short description of work done
PR Checklist
Changes
Issues
Closes #192