Skip to content
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

Merged
merged 25 commits into from
Apr 5, 2024
Merged

Tests/send payment #1206

merged 25 commits into from
Apr 5, 2024

Conversation

piyalbasu
Copy link
Contributor

@piyalbasu piyalbasu commented Mar 21, 2024

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" :)

@piyalbasu piyalbasu changed the title Tests/send payment 2 Tests/send payment Mar 21, 2024
import { login, PASSWORD } from "./helpers/login";

test("Send XLM payment", async ({ page, extensionId }) => {
await login({ page, extensionId });
Copy link
Contributor Author

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

Copy link
Contributor

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.

await page.getByRole("button", { name: "Fund with Friendbot" }).click();

await expect(page.getByTestId("account-assets")).toBeVisible({
timeout: 10000,
Copy link
Contributor

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.

Copy link
Contributor Author

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

@piyalbasu piyalbasu merged commit e9441c8 into master Apr 5, 2024
7 checks passed
@piyalbasu piyalbasu deleted the tests/send-payment-2 branch April 5, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants