-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: E2E real data #172
Conversation
// Reusable function for checking address | ||
async function checkAddress(page: Page, expectedAddress: string) { | ||
const address = await page.getByTestId("address").textContent(); | ||
expect(address).toBe(trim(expectedAddress)); |
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 think it's better to remove expect
condition from here and rename this helper function to regular getter. First of all this is more declarative way of writing tests + this function violates the single responsibility principle (it's a getter and expect function at the same time)
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.
Maybe we should wrap up all these helper functions into fixtures? Is there any example of using helpers
in there docs? I think that fixtures is more PlayWright way
@@ -16,27 +16,32 @@ | |||
"sort-imports": "eslint --fix .", | |||
"test": "jest", | |||
"test:watch": "jest --watch", | |||
"build:btcwallet:mainnet": "webpack --config webpack.btcWallet.config.js --env NEXT_PUBLIC_NETWORK=mainnet", |
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 looks weird. Instead of putting everything into our DApp application we could create a separate repo for test wallet and deploy it to npm. We could create a very simple browser extension for test purposes and inject it as devDependency into our main application.
I like the Tomo way of doing thing, they have created a separate repo even for their (our) wallet providers, what makes much easier to maintain their SDK and Wallet Providers
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.
Yep, as I mentioned on the call, we now have simple-staking
and a lot of stuff to build the wallet for e2e purposes.
Question though is are we ready to extract wallet providers and all of that outside of simple-staking? Is it a good time now or should we do it later?
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.
All this stuff around wallet providers is so confusing for me right now... If we have mobile/injectable provider in our app why just not use it for the test wallet? We can implement this interface in a separate repo and manually inject it into global window object before we run tests
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.
If I'm getting things right here is the list of packages and modules which we need extract to a new repo
import "core-js/web/url";
import * as ecc from "@bitcoin-js/tiny-secp256k1-asmjs";
import * as bip32 from "@scure/bip32";
import * as bip39 from "@scure/bip39";
import * as bitcoin from "bitcoinjs-lib";
import ECPairFactory from "ecpair";
import { getNetworkFees, getTipHeight } from "@/utils/mempool_api";
import { getPublicKeyNoCoord, isTaproot } from "@/utils/wallet";
import {
Fees,
InscriptionIdentifier,
Network,
UTXO,
WalletProvider,
} from "@/utils/wallet/wallet_provider";
import { nativeSegwitMainnetUTXOs } from "../mock/mainnet/nativeSegwit/utxos";
import { taprootMainnetUTXOs } from "../mock/mainnet/taproot/utxos";
import { nativeSegwitSignetUTXOs } from "../mock/signet/nativeSegwit/utxos";
import { taprootSignetUTXOs } from "../mock/signet/taproot/utxos";```
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.
If we have mobile/injectable provider in our app why just not use it for the test wallet
I did not find the solution for the wallet to be "as it is" in typescript code and we are just injecting the stuff into window.btcwallet that works in node.js and Chrome (other browsers) Playwright env. As soon as we are starting to say "bitcoinjs sign this" or "bip39/32 get the private key" or "mempool fetch the data" it will cause problems
The only solution to this issue that I found is to build a JS wallets packages (build) that have everything they need. Then playwright injects it and calls needed methods
Closed in favor of #193 |
Added real wallet E2E tests
mainnet/signet
andtaproot/native segwit
window.btcwallet
is something we need to inject, and we do want our transactions to be real, we have to build our wallets with all the necessary bitcoin libs. Currently, I usewebpack
for thatmocking
part is the initialUTXOs
andPOST
requests. I.e. under the hood real BTC signatures are happening.env.local
that you use. If you want to use a specificmainnet
andsignet
env variables, you need to create.env.mainnet
and.env.signet
.balance and address
test, we can connect bothsignet
andmainnet
wallets and check the stuff. But anything with thesigning
involved will require specific env variables. You cannot usemainnet
env variables with global parameters expectingX
mainnet
BTC Tip height onsignet
wallet. Sincesignet
tip height is going to be differentSo
staking,
unbonding,
withdrawaluse a set of mapping and test required network wallets only. That's because in our app we use
src/app/context/mempool/BtcHeightProvider.tsxwhich will give
Xwhile wallet mempool endpoint might give
Ydepending on the
env` variables.Besides reviewing the code I expect you run the instructions in
Readme.md
:npm i
<- install the dependenciesnpx playwright install
<- this is importantnpm run build:btcwallet
<- to build both walletsnpm run test:e2e
<- for your.env.local
ornpm run test:e2e:full
<- for your.env.mainnet
and.env.signet
Added an E2E section in Readme.md
Video: https://youtu.be/jppgJa9L6gM
Closes:
Partly closes, so the PR is not even bigger: