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

E2E Test Claims #215

Closed
0x4007 opened this issue Mar 29, 2024 · 21 comments · Fixed by #225
Closed

E2E Test Claims #215

0x4007 opened this issue Mar 29, 2024 · 21 comments · Fixed by #225

Comments

@0x4007
Copy link
Member

0x4007 commented Mar 29, 2024

I feel that it's really a coin toss to get claims to work on mobile. Anybody else having issues? We should really consider an end to end test with cypress etc because this is getting a bit annoying.

image

Originally posted by @pavlovcik in ubiquity/ubiquibot#924 (comment)

@0x4007
Copy link
Member Author

0x4007 commented Mar 29, 2024

@gentlementlegen can you set a time estimate that was a best guess

@gentlementlegen
Copy link
Member

@pavlovcik 4 hours seems fair, there is a lot of use cases to cover. What worries me is that even testing with Cypress doesn't really represent a true mobile device scenario because you are still running in the context of a desktop browser, but would definitely reduce the regressions that we are experiencing.

@0x4007
Copy link
Member Author

0x4007 commented Mar 30, 2024

a429a63

Works now from that commit but seems unrelated so I'm guessing the RPC stability makes this issue intermittent

Another solution might be to figure out how to host our own RPC endpoints (might need to run full nodes which seems like expensive and burdensome infrastructure to maintain)

Ideally there's some free solution like what Cloudflare was offering with its Ethereum RPC endpoints (but now doesn't seem to work)

image

@Keyrxng
Copy link
Contributor

Keyrxng commented Mar 30, 2024

/start

Copy link

ubiquibot bot commented Mar 30, 2024

DeadlineSat, Mar 30, 2:30 PM UTC
Registered Wallet 0xAe5D1F192013db889b1e2115A370aB133f359765
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@Keyrxng
Copy link
Contributor

Keyrxng commented Apr 17, 2024

/stop

E2E tests covering mobile is not quite feasible due to mobile browsers not supporting window.ethereum with the exception of bespoke wallet integrated browsers that requires manual testing unless we build MM from source and hack together E2E tests that way.

I was able to implement E2E tests for mobile with WalletConnect as the wallet provider with intercepts although cypress doesn't handle deep links/universal links well (at all) so I was just simulating a valid signer after modal interaction and not the wallet behaviours. IIUC 3rd party modals and external providers are to be avoided

Cypress is only simulating mobile-like dimensions and if traditional mobile browsers don't support window.ethereum then #220 displaying a permanent toast should be the default outcome for traditional mobile browsers.

The E2E desktop test would also cover bespoke mobile browsers (in this case I'm only referring to MM) as I've found that the MM browser uses window.ethereum when testing to see if it was MIPD compliant so really the E2E tests would look like:

  1. a successful E2E claim
  2. a successful permanent toast on non-web3 browsers (traditional mobile)
  3. the various outcomes for failures (rpc, declined tx, etc...)

Plus I'm an IOS user without a Mac (so I'm at a disadvantage for debugging) and if building the MM browser from source is opted for then that rules me out


Happy to release this if someone else would like a crack at it but also happy to pick it back up again depending on which way things go

@ubiquibot ubiquibot bot unassigned Keyrxng Apr 17, 2024
Copy link

ubiquibot bot commented Apr 17, 2024

+ You have been unassigned from the task

@gentlementlegen
Copy link
Member

@Keyrxng thanks for spending time in this. I think it would be already very helpful to have use cases covering:

  • simulating no wallet available from the browser
  • simulating network faults

That would cover 95% of our use cases and issues we encounter. As you mentioned having an emulator would make the tests more robust and accurate but I think it's quite an overkill and waste of time to cover 100%. Even with an emulator we cannot test every different browser there is.

@0x4007
Copy link
Member Author

0x4007 commented Apr 18, 2024

we cannot test every different browser there is.

Only need chrome and the default mobile browsers I suppose.

@gentlementlegen
Copy link
Member

@0x4007 was meaning on a mobile device where you have very specific browsers that support wallets (MetaMask, Coinbase etc.)

@0x4007
Copy link
Member Author

0x4007 commented Apr 19, 2024

I understand. Maybe we should wait until we make our own account abstraction solution. Then we only need to make it work for the latest default mobile browsers. I think the web3 browsers (especially the mobile ones are a joke) are all pretty terrible and unusable

However I think since metamask is the industry standard we should ensure 100% coverage with every metamask?

Copy link

ubiquibot bot commented May 3, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented May 3, 2024

[ 33 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueSpecification10
IssueComment433
Conversation Incentives
CommentFormattingRelevanceReward
> I feel that it's really a coin toss to get claims to work on m...
-1-
@gentlementlegen can you set a time estimate that was a best gue...
2.40.772.4
https://github.com/ubiquity/pay.ubq.fi/commit/a429a6345e425c949c...
160.6216
> we cannot test every different browser there is.

Only need ch...

20.722
I understand. Maybe we should wait until we make our own account...
12.60.7112.6

[ 24 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueComment317.6
ReviewComment16.4
Conversation Incentives
CommentFormattingRelevanceReward
@pavlovcik 4 hours seems fair, there is a lot of use cases to co...
5.40.875.4
@Keyrxng thanks for spending time in this. I think it would be a...
10.3
li:
  count: 2
  score: "2"
  words: 10
0.6310.3
@0x4007 was meaning on a mobile device where you have very speci...
1.90.611.9
I do not know now if that is related to my `env` but this is the...
6.4
code:
  count: 2
  score: "2"
  words: 4
0.526.4

[ 41.8 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
ReviewComment641.8
Conversation Incentives
CommentFormattingRelevanceReward
@gentlementlegen I'd appreciate any comments on how to improve t...
22.8
li:
  count: 2
  score: "0.5"
  words: 25
code:
  count: 2
  score: "0.5"
  words: 4
0.622.8
All tests pass for me which is curious.

Can I ask which two f...

3.20.53.2
Appreciated mate.

I do not know now if that is related to m...

10.75

code:
  count: 5
  score: "1.25"
  words: 12
0.6610.75
Successful CI run
1.85
li:
  count: 1
  score: "0.25"
  words: 13
0.751.85
ready when you are @gentlementlegen ...
0.50.630.5
If this could be merged in today that would be amazing cheers te...
2.70.792.7

@Keyrxng
Copy link
Contributor

Keyrxng commented May 3, 2024

@rndquu I didn't realise the bot had kicked me a fortnight ago

@Keyrxng
Copy link
Contributor

Keyrxng commented May 3, 2024

/start

Copy link

ubiquibot bot commented May 3, 2024

! Skipping '/start' because the issue is closed.

@rndquu rndquu reopened this May 3, 2024
@Keyrxng
Copy link
Contributor

Keyrxng commented May 3, 2024

/start

Copy link

ubiquibot bot commented May 3, 2024

Warning! This task was created over 34 days ago. Please confirm that this issue specification is accurate before starting.
DeadlineFri, May 3, 5:32 PM UTC
Registered Wallet 0xAe5D1F192013db889b1e2115A370aB133f359765
Tips:
  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@rndquu rndquu closed this as completed May 3, 2024
Copy link

ubiquibot bot commented May 3, 2024

+ Evaluating results. Please wait...

@Keyrxng
Copy link
Contributor

Keyrxng commented May 3, 2024

Appreciate you dude ty, have a good weekend

Copy link

ubiquibot bot commented May 3, 2024

[ 33 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueSpecification10
IssueComment433
Conversation Incentives
CommentFormattingRelevanceReward
> I feel that it's really a coin toss to get claims to work on m...
-1-
@gentlementlegen can you set a time estimate that was a best gue...
2.40.7852.4
https://github.com/ubiquity/pay.ubq.fi/commit/a429a6345e425c949c...
160.62516
> we cannot test every different browser there is.

Only need ch...

20.6752
I understand. Maybe we should wait until we make our own account...
12.60.4112.6

[ 24 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueComment317.6
ReviewComment16.4
Conversation Incentives
CommentFormattingRelevanceReward
@pavlovcik 4 hours seems fair, there is a lot of use cases to co...
5.40.8355.4
@Keyrxng thanks for spending time in this. I think it would be a...
10.3
li:
  count: 2
  score: "2"
  words: 10
0.6510.3
@0x4007 was meaning on a mobile device where you have very speci...
1.90.541.9
I do not know now if that is related to my `env` but this is the...
6.4
code:
  count: 2
  score: "2"
  words: 4
0.436.4

[ 349.3 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueTask1300
IssueComment20
ReviewComment649.3
Conversation Incentives
CommentFormattingRelevanceReward
@rndquu I didn't realise the bot had kicked me a fortnight ago...
-0.205-
Appreciate you dude ty, have a good weekend...
-0.125-
@gentlementlegen I'd appreciate any comments on how to improve t...
25.8
li:
  count: 2
  score: "2"
  words: 25
code:
  count: 2
  score: "2"
  words: 4
0.4125.8
All tests pass for me which is curious.

Can I ask which two f...

3.20.373.2
Appreciated mate.

I do not know now if that is related to m...

14.5

code:
  count: 5
  score: "5"
  words: 12
0.6114.5
Successful CI run
2.6
li:
  count: 1
  score: "1"
  words: 13
0.672.6
ready when you are @gentlementlegen ...
0.50.550.5
If this could be merged in today that would be amazing cheers te...
2.70.812.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants