-
Notifications
You must be signed in to change notification settings - Fork 29
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
Tests/send payment #1206
Tests/send payment #1206
Conversation
…nto tests/send-payment-2
import { login, PASSWORD } from "./helpers/login"; | ||
|
||
test("Send XLM payment", async ({ page, extensionId }) => { | ||
await login({ page, extensionId }); |
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.
Unfortunately, I also couldn't run this login
function in a before
hook. The background state does not seem to carry over. When I would run the test, it would just take me right back to onboarding.
Maybe the solution here is to run one big test that logs in, sends a payment, does a swap, and then checks account history in one swoop? Again, not an amazing solution - but at the very least one way to give us a little coverage while we figure out how to make these tests better
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.
Hmm interesting, yeah I mean on the one hand I think not relying on mocks makes for a better test but yeah it is definitely more involved. I find it weird that the background scope can't be mocked or retained though. Is this on manifest v2? I might expect this on v3 since it's service workers and they go away periodically but find it weird for v2.
extension/e2e-tests/helpers/login.ts
Outdated
await page.getByRole("button", { name: "Fund with Friendbot" }).click(); | ||
|
||
await expect(page.getByTestId("account-assets")).toBeVisible({ | ||
timeout: 10000, |
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.
Are these timeouts to wait for the network? I wonder if that would make them flaky at times?
Is there something else we can wait for, like maybe an await page.getByText("<Some title after the page loads?>")
Maybe this is fine if the timeout is long enough also, 10 seconds should cover long tail latency I would think.
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.
Yeah, this was to deal with Horizon requests sometimes taking a long time to resolve. This one, for example, is waiting for the above Fund With Friendbot
action to complete and for the assets to actually populate on screen.
I had run into a couple instances of the tests timing out before Horizon would come back with a response. I wanted to keep the timeout pretty long to give the network request plenty of time to resolve. If it takes longer than 10 seconds, it may be a sign that something is up with Horizon
Interesting development here: it does not seem possible to mock a function in a background script in Playwright. My original plan was to mock
chromse.storage.local.get
and return a logged in state, so upon loading the app, it would think the user already had an account setup without having to login. After spending a lot of time trying to get this to work, I was unsuccessful.So it seems we must run a "login" function before we can do anything related to accounts. I've added a helper that's similar to the onboarding "import 12 word phrase" test. In addition to generating a new account, it also funds it with friendbot so we have some XLM to try and send.
Having to generate a new account, import it, and then fund it is not a great solution as it takes some additional time before you even get to the functionality you're trying to test. But, this does at least give us some test coverage for changing networks, funding an account with friendbot, showing an XLM balance, and then sending an XLM payment which is really quite valuable in the interim. Hopefully we can iterate on this and make this a little more modular.
I'm trying not to let "perfect" be the enemy of "good" :)