multichain testing improvements #10713
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
c49d585
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6a91c0c0.agoric-sdk.pages.dev |
| Branch Preview URL: | https://ta-dry-e2e-tools.agoric-sdk.pages.dev |
| @@ -67,7 +67,8 @@ const queryAccountBalances = test.macro({ | |||
| }); | |||
|
|
|||
| const offerResult = await retryUntilCondition( | |||
| () => vstorageClient.queryData(`published.wallet.${agoricAddr}`), | |||
| () => smartWalletKit.readPublished(`wallet.${agoricAddr}`), | |||
There was a problem hiding this comment.
in what way is this a kit? it looks like 1 object with methods, not a kit of facets.
There was a problem hiding this comment.
i'm pretty sure "kit" is not necessarily an ExoClassKit.
is there another name you'd suggest? It would be a change to client-utils
| // @ts-expect-error UpdateRecord may not have 'status' | ||
| ({ status }) => status.id === offerId && (status.result || status.error), |
There was a problem hiding this comment.
if there's no status, status.id will throw, right?
| // @ts-expect-error UpdateRecord may not have 'status' | |
| ({ status }) => status.id === offerId && (status.result || status.error), | |
| (u) => u?.status.id === offerId && (u?.status.result || u?.status.error), |
There was a problem hiding this comment.
Have you considered the seatLike .getOfferResult() API?
There was a problem hiding this comment.
haven't considered that yet. I was just trying to DRY out what I could.
do you think that API should be part of client-utils?
| @@ -146,7 +148,8 @@ const queryAccountBalance = test.macro({ | |||
| }); | |||
|
|
|||
| const offerResult = await retryUntilCondition( | |||
| () => vstorageClient.queryData(`published.wallet.${agoricAddr}`), | |||
| () => smartWalletKit.readPublished(`wallet.${agoricAddr}`), | |||
| // @ts-expect-error UpdateRecord may not have 'status' | |||
There was a problem hiding this comment.
here we are with another copy of the getOfferResult() logic.
| @@ -200,14 +202,16 @@ const queryChainWithoutICQ = test.macro({ | |||
| }); | |||
|
|
|||
| const offerResult = await retryUntilCondition( | |||
| () => vstorageClient.queryData(`published.wallet.${agoricAddr}`), | |||
| () => smartWalletKit.readPublished(`wallet.${agoricAddr}`), | |||
There was a problem hiding this comment.
yeah. I think SmartWalletKit is a good place to hang something like "await a matching state of the status node and return it"
| ? Array<[string, Brand]> | ||
| : T extends 'fastUsdc' | ||
| ? ContractRecord | ||
| : T extends 'fastUsdc.poolMetrics' |
There was a problem hiding this comment.
This coupling still seems odd to me.
To my mind, fast-usdc is in the agoric-sdk for logistical reasons; any 3rd party developer should be able to make a similar app. Do we expect 3rd-party developers to update client-utils for every vstorage path they introduce?
Is there any way to flip it around so that a dev can combine types from agoricNames, inter protocol, and fast-usdc? I did something like that once...
agoric-sdk/packages/boot/tools/supports.ts
Lines 60 to 66 in 9418efc
There was a problem hiding this comment.
Do we expect 3rd-party developers to update client-utils for every vstorage path they introduce?
No.
Is there any way to flip it around…
Sure. It's just not anywhere near the top of priorities.
Let's tackle it when it's a problem. It won't be hard to fix but having the actual instance of the problem will make it more efficient than doing it now. (One of the "less effort later" kind of tasks.)
refs: #10578
Description
Some tangential improvements while working on #10578
Security Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations
Upgrade Considerations