Refactoring ethereum-on.spec.js to use fixtures#10778
Refactoring ethereum-on.spec.js to use fixtures#10778NiranjanaBinoy merged 3 commits intodevelopfrom
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Gudahtt
left a comment
There was a problem hiding this comment.
Looks good! I left a few comments about steps that can be removed entirely, but aside from that this seems functionally correct.
|
|
||
| assert.equal(await switchedNetworkDiv.getText(), '3'); | ||
| assert.equal(await chainIdDiv.getText(), '0x3'); | ||
| assert.equal(await accountsDiv.getText(), publicAddress.toLowerCase()); |
There was a problem hiding this comment.
I've just noticed that this last assertion doesn't really work. It's meant to test the accountsChanged event, but, that event is never emitted here and this still works 🤔
Not a blocker for this PR since it's an existing problem in the test, but we should move this out into its own it block at some point, where we compare this text value before and after switching accounts.
There was a problem hiding this comment.
I have not made any change to the last assertion as of yet, will handle that as a separate PR once after the current one is merged.
Gudahtt
left a comment
There was a problem hiding this comment.
I suspect we don't need any of these delay calls. Those were added a long time ago when we were directly using the selenium webdriver. Since then we've switched to our own driver abstraction, which is somewhat more robust.
The delay after switching to the |
Builds ready [557657c]
Page Load Metrics (650 ± 109 ms)
|
Fixes: #10771
Explanation:
metamask-extension/test/e2e/testsethereum-on.spec.jsto use fixtures.ethereum-on.spec.jstoprovider-events.spec.jsprovider-events.spec.jsis using the same fixture as thepersonal-signpersonal-signfolder for fixture toconnected-state