Run NFTs e2e tests on a controlled wallet address #3487
Conversation
Previously we run tests by importing `bravonaver.eth` address as a read-only account. We don't control that address and can't guarantee that it's state won't change (changes may lead to unstable tests). Now we're switching to importing the third address of the `testertesting.eth` account (`0x6f1b1f1feb01235e15a7962f16c389c7f8218ed6`) as a read-only. We control this address and can decide on its state.
|
I re-run the workflow with tests a couple of times to make sure it's not randomly failing - all executions were green: https://github.com/tahowallet/extension/actions/runs/5308837549. |
| @@ -36,10 +36,30 @@ test.describe("NFTs", () => { | |||
| } | |||
| }) | |||
|
|
|||
| await walletPageHelper.onboarding.addReadOnlyAccount("bravonaver.eth") | |||
| await walletPageHelper.onboarding.addReadOnlyAccount( | |||
| "0x6f1b1f1feb01235e15a7962f16c389c7f8218ed6" | |||
There was a problem hiding this comment.
Do we plan to reuse that address for other e2e tests? Then we can make it a variable.
There was a problem hiding this comment.
I don't have a particular test in mind yet where we would reuse it. Let's leave it as it is for now - if we introduce the test which uses the same account, we can parametrize this then.
There was a problem hiding this comment.
We should put it in a named constant regardless—no magic strings please!
e2e-tests/nfts.spec.ts
Outdated
| await page.getByTestId("top_menu_network_switcher").last().click() | ||
| await page.getByText(/^Optimism$/).click() | ||
| await expect( | ||
| page.getByTestId("top_menu_network_switcher").last() | ||
| ).toHaveText(/^Optimism$/) |
There was a problem hiding this comment.
What do you think about making it a util function where you just specify what network you want to switch to?
There was a problem hiding this comment.
Sounds good, I'll add this in this PR once I finish other work I'm doing rn.
There was a problem hiding this comment.
Done in 602e12b.
| @@ -70,7 +90,9 @@ test.describe("NFTs", () => { | |||
|
|
|||
| await page | |||
| .getByTestId("nft_account_filters") | |||
| .filter({ hasText: "bravonaver.eth" }) | |||
| .filter({ | |||
| hasText: /^(Phoenix|Matilda|Sirius|Topa|Atos|Sport|Lola|Foz)$/, | |||
There was a problem hiding this comment.
Does it make sense to buy/mint maybe a unstoppable domains address for this account so we can use it as a label?
There was a problem hiding this comment.
Yeah, good point, displaying the label based on UNS/ENS is a functionality related to NFTs, so it makes sense to test it in this test. Will do.
There was a problem hiding this comment.
I thought I will be able to buy the domain myself, but had to ask Ops. Let's maybe not wait for that to get processed and merge the PR as it is... I'll update the account name in a separate PR once we have the domain secured - wdyt?
## What's Changed * Repeater Connection: Forward requests to new default when default is switched off during dApp connection flow by @Shadowfiend in #3462 * Auto-Not-So-Matic: Fix two matic.network URLs to polygon.technology by @Shadowfiend in #3483 * v0.38.0 by @kkosiorowska in #3480 * Case Dismissed: Forcibly show DAppConnectionInfoBar popover on first dApp connection by @Shadowfiend in #3464 * Add Hardhat Fork Functionality by @0xDaedalus in #3247 * Faded Jeans: Rename fadeIn class to fade_in by @Shadowfiend in #3485 * Fix issue for discovery transaction hash by @kkosiorowska in #3458 * Full Sweep: Drop the USE_UPDATED_SIGNING_UI feature flag by @Shadowfiend in #3475 * Token Discovery - Remap redux asset balances by @hyphenized in #3195 * Make specific warnings for adding custom asset by @kkosiorowska in #3478 * Run NFTs e2e tests on a controlled wallet address by @michalinacienciala in #3487 * v0.38.1 by @jagodarybacka in #3484 **Full Changelog**: v0.38.1...v0.39.0 Latest build: [extension-builds-3496](https://github.com/tahowallet/extension/suites/13792957673/artifacts/764738599) (as of Thu, 22 Jun 2023 14:27:32 GMT).
Previously we run tests by importing
bravonaver.ethaddress as a read-only account. We don't control that address and can't guarantee that it's state won't change (changes may lead to unstable tests). Now we're switching to importing the third address of thetestertesting.ethaccount (0x6f1b1f1feb01235e15a7962f16c389c7f8218ed6) as a read-only. We control this address and can decide on its state.Latest build: extension-builds-3487 (as of Tue, 20 Jun 2023 20:39:00 GMT).