tests: approvals tests #628
Conversation
mateuszjasiuk
force-pushed
the
tests/approvals-handlers-test
branch
from
1763941 to
9524070
Compare
|
|
||
| // TODO: can tabId be 0? |
There was a problem hiding this comment.
I think potentially yes. At least, on MDN I didn't find anything that said it can't be 0:
id Optional
integer. The tab's ID. Tab IDs are unique within a browser session. The tab ID may also be set to tabs.TAB_ID_NONE for browser windows that don't host content tabs (for example, devtools windows).
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/Tab#id
| @@ -100,6 +100,7 @@ export class ApprovalsService { | |||
| if (!data) { | |||
| throw new Error(`Signing data for ${msgId} not found!`); | |||
| } | |||
| //TODO: Shouldn't we _clearPendingSignature when throwing? | |||
There was a problem hiding this comment.
@jurevans can say for sure 😄
There was a problem hiding this comment.
Probably? I don't think it would do anything for Signing data for ${msgId} not found!, but on the other errors, it should probably be there.
There was a problem hiding this comment.
I will resolve those todos later ;)
| @@ -127,13 +127,17 @@ export class ConnectInterfaceResponseMsg extends Message<void> { | |||
| } | |||
|
|
|||
| validate(): void { | |||
| if (!this.interfaceTabId) { | |||
There was a problem hiding this comment.
There seem to be a few instances of these falsy checks that typecheck even when they're slightly wrong. I'm wondering if it might be worth adding a linting rule in the future to prevent that.
There was a problem hiding this comment.
Yeah either that or we can just use io-ts also for those checks
| const signer = "signer"; | ||
| const signaturePromise = service.approveSignature(signer, "data"); | ||
|
|
||
| await new Promise((resolve) => |
There was a problem hiding this comment.
What does waiting here do?
There was a problem hiding this comment.
So the reason is approveSignature is async but it does not resolve the promise by itself.
Because of that I did not add await in front of the call as that would make test hang.
Instead we have to wait a "tick" so resolverMap will be filled and submitSignature will be able to resolve successfully 😅 It's a bit confusing.
| source: "source", | ||
| target: "target", | ||
| token: "token", | ||
| amount: BigNumber(100), |
There was a problem hiding this comment.
That's cool - I didn't know new was optional for BigNumber 😄
There was a problem hiding this comment.
Me neither :D
* test: approvals handler test * test: approvals messages and service tests * test: approvals service test cd
No description provided.